From 975bb7366681951751855b3f639b7828ff299c17 Mon Sep 17 00:00:00 2001 From: chrchr Date: Wed, 15 Nov 2023 16:18:04 +0100 Subject: [PATCH 1/3] Fix #12188 FN uninitvar with increment of struct member --- lib/astutils.cpp | 17 +++++++++++++++++ lib/astutils.h | 2 ++ lib/checkuninitvar.cpp | 12 ------------ test/testuninitvar.cpp | 15 +++++++++++++++ 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 0aaf6944e99..dcf79c8d132 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -3263,6 +3263,18 @@ static ExprUsage getFunctionUsage(const Token* tok, int indirect, const Settings return ExprUsage::Inconclusive; } +bool isLeafDot(const Token* tok) +{ + if (!tok) + return false; + const Token * parent = tok->astParent(); + if (!Token::simpleMatch(parent, ".")) + return false; + if (parent->astOperand2() == tok && !Token::simpleMatch(parent->astParent(), ".")) + return true; + return isLeafDot(parent); +} + ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings, bool cpp) { const Token* parent = tok->astParent(); @@ -3283,6 +3295,11 @@ ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings, !parent->isUnaryOp("&") && !(astIsRHS(tok) && isLikelyStreamRead(cpp, parent))) return ExprUsage::Used; + if (isLeafDot(tok)) { + const Token* top = parent->astTop(); + if (Token::Match(top, "%assign%|++|--") && top->str() != "=") + return ExprUsage::Used; + } if (Token::simpleMatch(parent, "=") && astIsRHS(tok)) { const Token* const lhs = parent->astOperand1(); if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs == lhs->variable()->nameToken()) diff --git a/lib/astutils.h b/lib/astutils.h index c5fe9e09577..b4a7be82c80 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -418,6 +418,8 @@ bool isCPPCast(const Token* tok); bool isConstVarExpression(const Token* tok, std::function skipPredicate = nullptr); +bool isLeafDot(const Token* tok); + enum class ExprUsage { None, NotUsed, PassedByReference, Used, Inconclusive }; ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings, bool cpp); diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index e0c8b7db615..94d64d27537 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1596,18 +1596,6 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string "$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, Certainty::normal); } -static bool isLeafDot(const Token* tok) -{ - if (!tok) - return false; - const Token * parent = tok->astParent(); - if (!Token::simpleMatch(parent, ".")) - return false; - if (parent->astOperand2() == tok && !Token::simpleMatch(parent->astParent(), ".")) - return true; - return isLeafDot(parent); -} - void CheckUninitVar::valueFlowUninit() { logChecker("CheckUninitVar::valueFlowUninit"); diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 0bd66b5cb1b..024f2b8eec0 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -7215,6 +7215,21 @@ class TestUninitVar : public TestFixture { " foo(&my_st);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("struct S {\n" // #12188 + " int i;\n" + "};\n" + "void f() {\n" + " S s;\n" + " ++s.i;\n" + "}\n" + "void g() {\n" + " S s;\n" + " s.i &= 3;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: s.i\n" + "[test.cpp:10]: (error) Uninitialized variable: s.i\n", + errout.str()); } void uninitvar_memberfunction() { From 6c953593bc5b1dd5d60576df881661634f8fad34 Mon Sep 17 00:00:00 2001 From: chrchr Date: Wed, 15 Nov 2023 16:22:22 +0100 Subject: [PATCH 2/3] Add test --- test/testuninitvar.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 024f2b8eec0..831764dc004 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -7225,10 +7225,15 @@ class TestUninitVar : public TestFixture { "}\n" "void g() {\n" " S s;\n" + " s.i--;\n" + "}\n" + "void h() {\n" + " S s;\n" " s.i &= 3;\n" "}\n"); ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: s.i\n" - "[test.cpp:10]: (error) Uninitialized variable: s.i\n", + "[test.cpp:10]: (error) Uninitialized variable: s.i\n" + "[test.cpp:14]: (error) Uninitialized variable: s.i\n", errout.str()); } From e17755402b843cbf6043bad61a91143a00a2f950 Mon Sep 17 00:00:00 2001 From: chrchr Date: Thu, 16 Nov 2023 14:42:52 +0100 Subject: [PATCH 3/3] Improve AST traversal --- lib/astutils.cpp | 6 ++++-- test/testuninitvar.cpp | 22 +++++++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index dcf79c8d132..43eb7d16567 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -3296,8 +3296,10 @@ ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings, !(astIsRHS(tok) && isLikelyStreamRead(cpp, parent))) return ExprUsage::Used; if (isLeafDot(tok)) { - const Token* top = parent->astTop(); - if (Token::Match(top, "%assign%|++|--") && top->str() != "=") + const Token* op = parent->astParent(); + while (Token::simpleMatch(op, ".")) + op = op->astParent(); + if (Token::Match(op, "%assign%|++|--") && op->str() != "=") return ExprUsage::Used; } if (Token::simpleMatch(parent, "=") && astIsRHS(tok)) { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 831764dc004..37b2100f547 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -7218,6 +7218,7 @@ class TestUninitVar : public TestFixture { valueFlowUninit("struct S {\n" // #12188 " int i;\n" + " struct T { int j; } t;\n" "};\n" "void f() {\n" " S s;\n" @@ -7230,10 +7231,25 @@ class TestUninitVar : public TestFixture { "void h() {\n" " S s;\n" " s.i &= 3;\n" + "}\n" + "void k() {\n" + " S s;\n" + " if (++s.i < 3) {}\n" + "}\n" + "void m() {\n" + " S s;\n" + " ++s.t.j;\n" + "}\n" + "void n() {\n" + " S s;\n" + " if (s.t.j-- < 3) {}\n" "}\n"); - ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: s.i\n" - "[test.cpp:10]: (error) Uninitialized variable: s.i\n" - "[test.cpp:14]: (error) Uninitialized variable: s.i\n", + ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: s.i\n" + "[test.cpp:11]: (error) Uninitialized variable: s.i\n" + "[test.cpp:15]: (error) Uninitialized variable: s.i\n" + "[test.cpp:19]: (error) Uninitialized variable: s.i\n" + "[test.cpp:23]: (error) Uninitialized variable: s.t.j\n" + "[test.cpp:27]: (error) Uninitialized variable: s.t.j\n", errout.str()); }