From c0a8d628b98ff0b65a02e603c496020edb7a0e82 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 22 Aug 2019 23:23:21 -0500 Subject: [PATCH] Fix issue 6010: Uninitialized inner struct (#2098) * Fix issue 6010: Uninitialized inner struct * Show to root variable that is unitialized * Warn on pointer dereferences --- lib/astutils.cpp | 20 ++++++++++++++++++++ lib/astutils.h | 2 ++ lib/checkuninitvar.cpp | 24 +++++++++++++++++++++--- lib/valueflow.cpp | 6 +++++- test/testuninitvar.cpp | 11 +++++++++++ 5 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 3544c2e19..c59d5531f 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -52,6 +52,26 @@ void visitAstNodes(const Token *ast, std::function *result, const char* op, nonneg int depth = 0) +{ + ++depth; + if (!tok || depth >= 100) + return; + if (tok->str() == op) { + astFlattenRecursive(tok->astOperand1(), result, op, depth); + astFlattenRecursive(tok->astOperand2(), result, op, depth); + } else { + result->push_back(tok); + } +} + +std::vector astFlatten(const Token* tok, const char* op) +{ + std::vector result; + astFlattenRecursive(tok, &result, op); + return result; +} + static bool astIsCharWithSign(const Token *tok, ValueType::Sign sign) { diff --git a/lib/astutils.h b/lib/astutils.h index 491677bc8..659679ca3 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -48,6 +48,8 @@ enum class ChildrenToVisit { */ void visitAstNodes(const Token *ast, std::function visitor); +std::vector astFlatten(const Token* tok, const char* op); + /** Is expression a 'signed char' if no promotion is used */ bool astIsSignedChar(const Token *tok); /** Is expression a 'char' if no promotion is used? */ diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 91fb60771..d3b8eea18 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1284,6 +1284,18 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string "$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE908, false); } +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) + return true; + return isLeafDot(parent); +} + void CheckUninitVar::valueFlowUninit() { const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); @@ -1299,8 +1311,6 @@ void CheckUninitVar::valueFlowUninit() } if (!tok->variable()) continue; - if (Token::Match(tok->astParent(), ". %var%") && tok->astParent()->astOperand1() == tok) - continue; auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue)); if (v == tok->values().end()) continue; @@ -1311,11 +1321,19 @@ void CheckUninitVar::valueFlowUninit() if (v->indirect > 1 || v->indirect < 0) continue; bool unknown; - if (v->indirect == 1 && !CheckNullPointer::isPointerDeRef(tok, unknown, mSettings)) + const bool deref = CheckNullPointer::isPointerDeRef(tok, unknown, mSettings); + if (v->indirect == 1 && !deref) + continue; + if (Token::Match(tok->astParent(), ". %var%") && !(isLeafDot(tok) || deref)) continue; if (!Token::Match(tok->astParent(), ". %name% (") && isVariableChanged(tok, mSettings, mTokenizer->isCPP())) continue; uninitvarError(tok, tok->str(), v->errorPath); + const Token * nextTok = tok; + while(Token::simpleMatch(nextTok->astParent(), ".")) + nextTok = nextTok->astParent(); + nextTok = nextAfterAstRightmostLeaf(nextTok); + tok = nextTok ? nextTok : tok; } } } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d107c3958..572b6f8d7 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -461,9 +461,13 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti pvalue.indirect++; setTokenValue(parent, pvalue, settings); } else if (Token::Match(parent, ". %var%") && parent->astOperand1() == tok) { - if (parent->originalName() == "->") + if (parent->originalName() == "->" && pvalue.indirect > 0) pvalue.indirect--; setTokenValue(parent->astOperand2(), pvalue, settings); + } else if (Token::Match(parent->astParent(), ". %var%") && parent->astParent()->astOperand1() == parent) { + if (parent->astParent()->originalName() == "->" && pvalue.indirect > 0) + pvalue.indirect--; + setTokenValue(parent->astParent()->astOperand2(), pvalue, settings); } else if (parent->isUnaryOp("*") && pvalue.indirect > 0) { pvalue.indirect--; setTokenValue(parent, pvalue, settings); diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index ca9cb8106..6b41c6919 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -4004,6 +4004,17 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: flags\n", errout.str()); + valueFlowUninit("struct pc_data {\n" + " struct {\n" + " char * strefa;\n" + " } wampiryzm;\n" + "};\n" + "void f() {\n" + " struct pc_data *pcdata;\n" + " if ( *pcdata->wampiryzm.strefa == '\\0' ) { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (error) Uninitialized variable: pcdata\n", errout.str()); + valueFlowUninit("void f(bool * x) {\n" " *x = false;\n" "}\n"