Skip to content

Commit 9fb2625

Browse files
pfultz2pfultz2
authored
Fix 12247: FP uninitvar with pointer to uninitialized array (#6414)
1 parent 9990975 commit 9fb2625

5 files changed

Lines changed: 76 additions & 13 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2435,6 +2435,31 @@ static bool isArray(const Token* tok)
24352435
return false;
24362436
}
24372437

2438+
static bool isMutableExpression(const Token* tok)
2439+
{
2440+
if (!tok)
2441+
return false;
2442+
if (tok->isLiteral() || tok->isKeyword() || tok->isStandardType() || tok->isEnumerator())
2443+
return false;
2444+
if (Token::Match(tok, ",|;|:|]|)|}"))
2445+
return false;
2446+
if (Token::simpleMatch(tok, "[ ]"))
2447+
return false;
2448+
if (Token::Match(tok->previous(), "%name% (") && tok->previous()->isKeyword())
2449+
return false;
2450+
if (Token::Match(tok, "<|>") && tok->link())
2451+
return false;
2452+
if (Token::simpleMatch(tok, "[") && tok->astOperand1())
2453+
return isMutableExpression(tok->astOperand1());
2454+
if (const Variable* var = tok->variable()) {
2455+
if (var->nameToken() == tok)
2456+
return false;
2457+
if (!var->isPointer() && var->isConst())
2458+
return false;
2459+
}
2460+
return true;
2461+
}
2462+
24382463
bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings &settings, bool *inconclusive)
24392464
{
24402465
if (!tok)
@@ -2545,7 +2570,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
25452570

25462571
bool isVariableChanged(const Token *tok, int indirect, const Settings &settings, int depth)
25472572
{
2548-
if (!tok)
2573+
if (!isMutableExpression(tok))
25492574
return false;
25502575

25512576
if (indirect == 0 && isConstVarExpression(tok))
@@ -2594,12 +2619,12 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings &settings,
25942619
tok2 = skipRedundantPtrOp(tok2, tok2->astParent());
25952620

25962621
if (tok2->astParent() && tok2->astParent()->isAssignmentOp()) {
2597-
if (((indirect == 0 || tok2 != tok) || (indirect == 1 && tok2->str() == ".")) && tok2 == tok2->astParent()->astOperand1())
2622+
if (astIsLHS(tok2))
25982623
return true;
25992624
// Check if assigning to a non-const lvalue
26002625
const Variable * var = getLHSVariable(tok2->astParent());
2601-
if (var && var->isReference() && !var->isConst() &&
2602-
((var->nameToken() && var->nameToken()->next() == tok2->astParent()) || var->isPointer())) {
2626+
if (var && var->isReference() && !var->isConst() && var->nameToken() &&
2627+
var->nameToken()->next() == tok2->astParent()) {
26032628
if (!var->isLocal() || isVariableChanged(var, settings, depth - 1))
26042629
return true;
26052630
}
@@ -2815,7 +2840,7 @@ static bool isExpressionChangedAt(const F& getExprTok,
28152840
{
28162841
if (depth < 0)
28172842
return true;
2818-
if (tok->isLiteral() || tok->isKeyword() || tok->isStandardType() || Token::Match(tok, ",|;|:"))
2843+
if (!isMutableExpression(tok))
28192844
return false;
28202845
if (tok->exprId() != exprid || (!tok->varId() && !tok->isName())) {
28212846
if (globalvar && Token::Match(tok, "%name% (") && !(tok->function() && tok->function()->isAttributePure()))

‎lib/checkother.cpp‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,6 +1458,24 @@ static const Token* getVariableChangedStart(const Variable* p)
14581458
return start;
14591459
}
14601460

1461+
static bool isConstPointerVariable(const Variable* p, const Settings& settings)
1462+
{
1463+
const int indirect = p->isArray() ? p->dimensions().size() : 1;
1464+
const Token* start = getVariableChangedStart(p);
1465+
while (const Token* tok =
1466+
findVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, settings)) {
1467+
if (p->isReference())
1468+
return false;
1469+
// Assigning a pointer through another pointer may still be const
1470+
if (!Token::simpleMatch(tok->astParent(), "="))
1471+
return false;
1472+
if (!astIsLHS(tok))
1473+
return false;
1474+
start = tok->next();
1475+
}
1476+
return true;
1477+
}
1478+
14611479
namespace {
14621480
struct CompareVariables {
14631481
bool operator()(const Variable* a, const Variable* b) const {
@@ -1500,6 +1518,9 @@ void CheckOther::checkConstPointer()
15001518
!astIsRangeBasedForDecl(nameTok))
15011519
continue;
15021520
}
1521+
// Skip function pointers
1522+
if (Token::Match(nameTok, "%name% ) ("))
1523+
continue;
15031524
const ValueType* const vt = tok->valueType();
15041525
if (!vt)
15051526
continue;
@@ -1609,9 +1630,11 @@ void CheckOther::checkConstPointer()
16091630
continue;
16101631
}
16111632
if (std::find(nonConstPointers.cbegin(), nonConstPointers.cend(), p) == nonConstPointers.cend()) {
1612-
const Token *start = getVariableChangedStart(p);
1613-
const int indirect = p->isArray() ? p->dimensions().size() : 1;
1614-
if (isVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, *mSettings))
1633+
// const Token *start = getVariableChangedStart(p);
1634+
// const int indirect = p->isArray() ? p->dimensions().size() : 1;
1635+
// if (isVariableChanged(start, p->scope()->bodyEnd, indirect, p->declarationId(), false, *mSettings))
1636+
// continue;
1637+
if (!isConstPointerVariable(p, *mSettings))
16151638
continue;
16161639
if (p->typeStartToken() && p->typeStartToken()->isSimplifiedTypedef() && !(Token::simpleMatch(p->typeEndToken(), "*") && !p->typeEndToken()->isSimplifiedTypedef()))
16171640
continue;

‎test/testastutils.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,6 @@ class TestAstUtils : public TestFixture {
249249
"void f(int x) { g(&x); }\n",
250250
"{",
251251
"}"));
252-
253252
ASSERT_EQUALS(false, isVariableChanged("const int A[] = { 1, 2, 3 };", "[", "]"));
254253
}
255254

‎test/teststl.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2429,29 +2429,29 @@ class TestStl : public TestFixture {
24292429
"}");
24302430
ASSERT_EQUALS("[test.cpp:3]: (error) When ii==foo.size(), foo.at(ii) is out of bounds.\n", errout_str());
24312431

2432-
check("void foo(const std::string& foo) {\n"
2432+
check("void foo(std::string& foo) {\n"
24332433
" for (unsigned int ii = 0; ii <= foo.length(); ++ii) {\n"
24342434
" foo[ii] = 'x';\n"
24352435
" }\n"
24362436
"}");
24372437
ASSERT_EQUALS("[test.cpp:3]: (error) When ii==foo.size(), foo[ii] is out of bounds.\n", errout_str());
24382438

2439-
check("void foo(const std::string& foo, unsigned int ii) {\n"
2439+
check("void foo(std::string& foo, unsigned int ii) {\n"
24402440
" if (ii <= foo.length()) {\n"
24412441
" foo[ii] = 'x';\n"
24422442
" }\n"
24432443
"}");
24442444
ASSERT_EQUALS("[test.cpp:3]: (error) When ii==foo.size(), foo[ii] is out of bounds.\n", errout_str());
24452445

2446-
check("void foo(const std::string& foo, unsigned int ii) {\n"
2446+
check("void foo(std::string& foo, unsigned int ii) {\n"
24472447
" do {\n"
24482448
" foo[ii] = 'x';\n"
24492449
" ++i;\n"
24502450
" } while(ii <= foo.length());\n"
24512451
"}");
24522452
ASSERT_EQUALS("[test.cpp:3]: (error) When ii==foo.size(), foo[ii] is out of bounds.\n", errout_str());
24532453

2454-
check("void foo(const std::string& foo, unsigned int ii) {\n"
2454+
check("void foo(std::string& foo, unsigned int ii) {\n"
24552455
" if (anything()) {\n"
24562456
" } else if (ii <= foo.length()) {\n"
24572457
" foo[ii] = 'x';\n"

‎test/testuninitvar.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6492,6 +6492,22 @@ class TestUninitVar : public TestFixture {
64926492
" f(i);\n"
64936493
"}\n");
64946494
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (warning) Uninitialized variable: r\n", errout_str());
6495+
6496+
// #12247
6497+
valueFlowUninit("void f() {\n"
6498+
" char a[10], *p = &a[0];\n"
6499+
" p = getenv(\"abc\");\n"
6500+
" printf(\"%\", p);\n"
6501+
"}\n");
6502+
ASSERT_EQUALS("", errout_str());
6503+
6504+
valueFlowUninit("void f(char *q) {\n"
6505+
" char a[1];\n"
6506+
" char *p = a;\n"
6507+
" p = q;\n"
6508+
" printf(\"%s\", p);\n"
6509+
"}\n");
6510+
ASSERT_EQUALS("", errout_str());
64956511
}
64966512

64976513
void valueFlowUninitBreak() { // Do not show duplicate warnings about the same uninitialized value

0 commit comments

Comments
 (0)