Skip to content

Commit 115f17c

Browse files
pfultz2pfultz2
authored
ValueFlow: Improve the starting point for uninitialized variables to find more uninitialized usages after many conditionals (#4930)
1 parent 16a9f54 commit 115f17c

3 files changed

Lines changed: 75 additions & 12 deletions

File tree

‎lib/valueflow.cpp‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7692,6 +7692,52 @@ static void addToErrorPath(ValueFlow::Value& value, const ValueFlow::Value& from
76927692
});
76937693
}
76947694

7695+
static std::vector<Token*> findAllUsages(const Variable* var, Token* start)
7696+
{
7697+
std::vector<Token*> result;
7698+
const Scope* scope = var->scope();
7699+
if (!scope)
7700+
return result;
7701+
Token* tok2 = Token::findmatch(start, "%varid%", scope->bodyEnd, var->declarationId());
7702+
while (tok2) {
7703+
result.push_back(tok2);
7704+
tok2 = Token::findmatch(tok2->next(), "%varid%", scope->bodyEnd, var->declarationId());
7705+
}
7706+
return result;
7707+
}
7708+
7709+
static Token* findStartToken(const Variable* var, Token* start)
7710+
{
7711+
std::vector<Token*> uses = findAllUsages(var, start);
7712+
if (uses.empty())
7713+
return start;
7714+
Token* first = uses.front();
7715+
if (Token::findmatch(start, "goto|asm|setjmp|longjmp", first))
7716+
return start;
7717+
const Scope* scope = first->scope();
7718+
// If there is only one usage or the first usage is in the same scope
7719+
if (uses.size() == 1 || scope == var->scope())
7720+
return first->previous();
7721+
// If all uses are in the same scope
7722+
if (std::all_of(uses.begin() + 1, uses.end(), [&](const Token* tok) {
7723+
return tok->scope() == scope;
7724+
}))
7725+
return first->previous();
7726+
// Compute the outer scope
7727+
while (scope && scope->nestedIn != var->scope())
7728+
scope = scope->nestedIn;
7729+
if (!scope)
7730+
return start;
7731+
Token* tok = const_cast<Token*>(scope->bodyStart);
7732+
if (!tok)
7733+
return start;
7734+
if (Token::simpleMatch(tok->tokAt(-2), "} else {"))
7735+
tok = tok->linkAt(-2);
7736+
if (Token::simpleMatch(tok->previous(), ") {"))
7737+
return tok->linkAt(-1)->previous();
7738+
return tok;
7739+
}
7740+
76957741
static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDatabase*/, const Settings* settings)
76967742
{
76977743
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
@@ -7718,6 +7764,8 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba
77187764

77197765
bool partial = false;
77207766

7767+
Token* start = findStartToken(var, tok->next());
7768+
77217769
std::map<Token*, ValueFlow::Value> partialReads;
77227770
if (const Scope* scope = var->typeScope()) {
77237771
if (Token::findsimplematch(scope->bodyStart, "union", scope->bodyEnd))
@@ -7733,7 +7781,7 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba
77337781
continue;
77347782
}
77357783
MemberExpressionAnalyzer analyzer(memVar.nameToken()->str(), tok, uninitValue, tokenlist, settings);
7736-
valueFlowGenericForward(tok->next(), tok->scope()->bodyEnd, analyzer, *settings);
7784+
valueFlowGenericForward(start, tok->scope()->bodyEnd, analyzer, *settings);
77377785

77387786
for (auto&& p : *analyzer.partialReads) {
77397787
Token* tok2 = p.first;
@@ -7763,7 +7811,7 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba
77637811
if (partial)
77647812
continue;
77657813

7766-
valueFlowForward(tok->next(), tok->scope()->bodyEnd, var->nameToken(), uninitValue, tokenlist, settings);
7814+
valueFlowForward(start, tok->scope()->bodyEnd, var->nameToken(), uninitValue, tokenlist, settings);
77677815
}
77687816
}
77697817

‎test/testuninitvar.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3499,7 +3499,7 @@ class TestUninitVar : public TestFixture {
34993499
" }\n"
35003500
"}",
35013501
"test.cpp");
3502-
ASSERT_EQUALS("", errout.str());
3502+
TODO_ASSERT_EQUALS("", "[test.cpp:6]: (error) Uninitialized variable: i\n", errout.str());
35033503

35043504
valueFlowUninit("void f() {\n"
35053505
" int i, y;\n"
@@ -3510,7 +3510,7 @@ class TestUninitVar : public TestFixture {
35103510
" }\n"
35113511
"}",
35123512
"test.cpp");
3513-
ASSERT_EQUALS("", errout.str());
3513+
TODO_ASSERT_EQUALS("", "[test.cpp:6]: (error) Uninitialized variable: i\n", errout.str());
35143514

35153515
valueFlowUninit("void f() {\n"
35163516
" int i, y;\n"
@@ -3838,7 +3838,7 @@ class TestUninitVar : public TestFixture {
38383838
" if (y == 1) { return; }\n"
38393839
" return x;\n"
38403840
"}");
3841-
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Uninitialized variable: x\n", errout.str());
3841+
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: x\n", errout.str());
38423842

38433843
valueFlowUninit("int f(int x) {\n"
38443844
" int ret;\n"
@@ -3871,15 +3871,15 @@ class TestUninitVar : public TestFixture {
38713871
" if (foo) break;\n"
38723872
" return x;\n"
38733873
"}");
3874-
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (error) Uninitialized variable: x\n", errout.str());
3874+
ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: x\n", errout.str());
38753875

38763876
valueFlowUninit("int f() {\n"
38773877
" int x;\n"
38783878
" while (foo)\n"
38793879
" if (bar) break;\n"
38803880
" return x;\n"
38813881
"}");
3882-
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (error) Uninitialized variable: x\n", errout.str());
3882+
ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: x\n", errout.str());
38833883

38843884
// try/catch : don't warn about exception variable
38853885
valueFlowUninit("void f() {\n"
@@ -6662,7 +6662,7 @@ class TestUninitVar : public TestFixture {
66626662
" struct AB ab;\n"
66636663
" while (x) { ab.a = ab.a + 1; }\n"
66646664
"}");
6665-
TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: ab.a\n", "", errout.str());
6665+
ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: ab.a\n", errout.str());
66666666

66676667
valueFlowUninit("struct AB { int a; };\n"
66686668
"void f() {\n"

‎test/testvalueflow.cpp‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ class TestValueFlow : public TestFixture {
162162
TEST_CASE(valueFlowSymbolicStrlen);
163163
TEST_CASE(valueFlowSmartPointer);
164164
TEST_CASE(valueFlowImpossibleMinMax);
165+
TEST_CASE(valueFlowImpossibleUnknownConstant);
165166
}
166167

167168
static bool isNotTokValue(const ValueFlow::Value &val) {
@@ -5433,18 +5434,19 @@ class TestValueFlow : public TestFixture {
54335434
" return x;\n"
54345435
"}\n";
54355436
values = tokenValues(code, "x ; }", ValueFlow::Value::ValueType::UNINIT);
5436-
ASSERT_EQUALS(0, values.size());
5437+
ASSERT_EQUALS(1, values.size());
5438+
ASSERT_EQUALS(true, values.front().isUninitValue());
54375439

5438-
code = "void f() {\n"
5440+
code = "void f(int x) {\n"
54395441
" int i;\n"
5440-
" if (x) {\n"
5442+
" if (x > 0) {\n"
54415443
" int y = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated
54425444
" if (y != 0) return;\n"
54435445
" i++;\n"
54445446
" }\n"
54455447
"}\n";
54465448
values = tokenValues(code, "i ++", ValueFlow::Value::ValueType::UNINIT);
5447-
ASSERT_EQUALS(0, values.size());
5449+
TODO_ASSERT_EQUALS(0, 1, values.size());
54485450
}
54495451

54505452
void valueFlowConditionExpressions() {
@@ -7874,6 +7876,19 @@ class TestValueFlow : public TestFixture {
78747876
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1));
78757877
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1));
78767878
}
7879+
7880+
void valueFlowImpossibleUnknownConstant()
7881+
{
7882+
const char* code;
7883+
7884+
code = "void f(bool b) {\n"
7885+
" if (b) {\n"
7886+
" int x = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated
7887+
" if (x != 0) return;\n"
7888+
" }\n"
7889+
"}\n";
7890+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, 0));
7891+
}
78777892
};
78787893

78797894
REGISTER_TEST(TestValueFlow)

0 commit comments

Comments
 (0)