From 5ab010fdd87d8c84ee8d09d7d1688b85d6c0782c Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 11 May 2022 12:24:33 -0500 Subject: [PATCH 1/7] Fix 11057: FP danglingTemporaryLifetime with reference member --- lib/astutils.cpp | 12 ++++++-- lib/astutils.h | 2 ++ lib/symboldatabase.cpp | 8 +++++ lib/symboldatabase.h | 3 ++ lib/valueflow.cpp | 70 +++++++++++++++++++++++++++++++----------- 5 files changed, 74 insertions(+), 21 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 8f7f9d9e113..8f6aff6cd06 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2603,6 +2603,14 @@ int numberOfArguments(const Token *start) return arguments; } +const Token* getArgumentStart(const Token* tok) +{ + const Token *startTok = tok->astOperand2(); + if (!startTok && tok->next() != tok->link()) + startTok = tok->astOperand1(); + return startTok; +} + std::vector getArguments(const Token *ftok) { const Token* tok = ftok; @@ -2610,9 +2618,7 @@ std::vector getArguments(const Token *ftok) tok = ftok->next(); if (!Token::Match(tok, "(|{|[")) return std::vector {}; - const Token *startTok = tok->astOperand2(); - if (!startTok && tok->next() != tok->link()) - startTok = tok->astOperand1(); + const Token *startTok = getArgumentStart(tok); return astFlatten(startTok, ","); } diff --git a/lib/astutils.h b/lib/astutils.h index a1bb11bff4d..feea3cf4666 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -310,6 +310,8 @@ bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive = nullptr) bool isAliased(const Variable *var); +const Token* getArgumentStart(const Token* tok); + /** Determines the number of arguments - if token is a function call or macro * @param start token which is supposed to be the function/macro name. * \return Number of arguments diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 504e49b6677..af872dd25a3 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7172,6 +7172,14 @@ MathLib::bigint ValueType::typeSize(const cppcheck::Platform &platform, bool p) return 0; } +bool ValueType::isTypeEqual(const ValueType* that) const +{ + auto tie = [](const ValueType* vt) { + return std::tie(vt->type, vt->container, vt->pointer, vt->typeScope, vt->smartPointer); + }; + return tie(this) == tie(that); +} + std::string ValueType::str() const { std::string ret; diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index c2731b81b05..c5eb887e70b 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -1334,6 +1334,9 @@ class CPPCHECKLIB ValueType { MathLib::bigint typeSize(const cppcheck::Platform &platform, bool p=false) const; + /// Check if type is the same ignoring const and references + bool isTypeEqual(const ValueType* that) const; + std::string str() const; std::string dump() const; }; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0ed20f28693..5081dec73f5 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -311,22 +311,36 @@ static std::vector getParentValueTypes(const Token* tok, } else if (Token::Match(tok->astParent(), "(|{|,")) { int argn = -1; const Token* ftok = getTokenArgumentFunction(tok, argn); - if (ftok && ftok->function()) { - std::vector result; - std::vector argsVars = getArgumentVars(ftok, argn); - const Token* nameTok = nullptr; - for (const Variable* var : getArgumentVars(ftok, argn)) { - if (!var) - continue; - if (!var->valueType()) - continue; - nameTok = var->nameToken(); - result.push_back(*var->valueType()); - } - if (result.size() == 1 && nameTok && parent) { - *parent = nameTok; + const Token* typeTok = nullptr; + if (ftok && argn >= 0) { + if (ftok->function()) { + std::vector result; + std::vector argsVars = getArgumentVars(ftok, argn); + const Token* nameTok = nullptr; + for (const Variable* var : getArgumentVars(ftok, argn)) { + if (!var) + continue; + if (!var->valueType()) + continue; + nameTok = var->nameToken(); + result.push_back(*var->valueType()); + } + if (result.size() == 1 && nameTok && parent) { + *parent = nameTok; + } + return result; + } else if(const Type* t = Token::typeOf(ftok, &typeTok)) { + if (astIsPointer(typeTok)) + return {*typeTok->valueType()}; + const Scope* scope = t->classScope; + // Check for aggregate constructors + if (scope && scope->numConstructors == 0 && t->derivedFrom.empty() && (t->isClassType() || t->isStructType())) { + assert(argn < scope->varlist.size()); + auto it = std::next(scope->varlist.begin(), argn); + if (it->valueType()) + return {*it->valueType()}; + } } - return result; } } if (settings && Token::Match(tok->astParent()->tokAt(-2), ". push_back|push_front|insert|push (") && @@ -3293,6 +3307,13 @@ static std::vector getLifetimeTokens(const Token* tok, return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1), !(astIsContainer(vartok) && Token::simpleMatch(vartok->astParent(), "["))); } + } else if (Token::simpleMatch(tok, "{") && getArgumentStart(tok) && !Token::simpleMatch(getArgumentStart(tok), ",") && getArgumentStart(tok)->valueType()) { + const Token* vartok = getArgumentStart(tok); + auto vts = getParentValueTypes(tok); + for(const ValueType& vt:vts) { + if (vt.isTypeEqual(vartok->valueType())) + return getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1); + } } return {{tok, std::move(errorPath)}}; } @@ -4216,7 +4237,15 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error } for (const ValueType& vt : vts) { - if (vt.container && vt.type == ValueType::CONTAINER) { + if (vt.pointer > 0) { + std::vector args = getArguments(tok); + LifetimeStore::forEach(args, + "Passed to initializer list.", + ValueFlow::Value::LifetimeKind::SubObject, + [&](const LifetimeStore& ls) { + ls.byVal(tok, tokenlist, errorLogger, settings); + }); + } else if (vt.container && vt.type == ValueType::CONTAINER) { std::vector args = getArguments(tok); if (args.size() == 1 && vt.container->view && astIsContainerOwned(args.front())) { LifetimeStore{args.front(), "Passed to container view.", ValueFlow::Value::LifetimeKind::SubObject} @@ -4238,7 +4267,12 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error }); } } else { - valueFlowLifetimeClassConstructor(tok, Token::typeOf(tok->previous()), tokenlist, errorLogger, settings); + const Type* t = nullptr; + if (vt.typeScope && vt.typeScope->definedType) + t = vt.typeScope->definedType; + else + t = Token::typeOf(tok->previous()); + valueFlowLifetimeClassConstructor(tok, t, tokenlist, errorLogger, settings); } } } @@ -4544,7 +4578,7 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger valueFlowForwardLifetime(parent->tokAt(2), tokenlist, errorLogger, settings); } // Check constructors - else if (Token::Match(tok, "=|return|%type%|%var% {") && !isScope(tok->next())) { + else if (Token::Match(tok, "=|return|%name%|{|,|> {") && !isScope(tok->next())) { valueFlowLifetimeConstructor(tok->next(), tokenlist, errorLogger, settings); } // Check function calls From c9360b963d26e96c82e1e5e62dcc63c6923a3da4 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 11 May 2022 12:28:58 -0500 Subject: [PATCH 2/7] Add test --- test/testautovariables.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 09c265f64f3..77b3ddd38a5 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -3563,6 +3563,16 @@ class TestAutoVariables : public TestFixture { " }\n" "};\n"); ASSERT_EQUALS("", errout.str()); + + // #11057 + check("struct S {\n" + " int& r;\n" + "};\n" + "void f(int i) {\n" + " const S a[] = { { i } };\n" + " for (const auto& s : a) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void danglingLifetimeBorrowedMembers() From b87a050c9d1f3335fe816b67d8f53ef319b562ba Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 11 May 2022 12:31:00 -0500 Subject: [PATCH 3/7] Format --- lib/astutils.cpp | 4 ++-- lib/valueflow.cpp | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 8f6aff6cd06..ec466ccc770 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2605,7 +2605,7 @@ int numberOfArguments(const Token *start) const Token* getArgumentStart(const Token* tok) { - const Token *startTok = tok->astOperand2(); + const Token* startTok = tok->astOperand2(); if (!startTok && tok->next() != tok->link()) startTok = tok->astOperand1(); return startTok; @@ -2618,7 +2618,7 @@ std::vector getArguments(const Token *ftok) tok = ftok->next(); if (!Token::Match(tok, "(|{|[")) return std::vector {}; - const Token *startTok = getArgumentStart(tok); + const Token* startTok = getArgumentStart(tok); return astFlatten(startTok, ","); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 5081dec73f5..af2733068e6 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -329,12 +329,13 @@ static std::vector getParentValueTypes(const Token* tok, *parent = nameTok; } return result; - } else if(const Type* t = Token::typeOf(ftok, &typeTok)) { + } else if (const Type* t = Token::typeOf(ftok, &typeTok)) { if (astIsPointer(typeTok)) return {*typeTok->valueType()}; const Scope* scope = t->classScope; // Check for aggregate constructors - if (scope && scope->numConstructors == 0 && t->derivedFrom.empty() && (t->isClassType() || t->isStructType())) { + if (scope && scope->numConstructors == 0 && t->derivedFrom.empty() && + (t->isClassType() || t->isStructType())) { assert(argn < scope->varlist.size()); auto it = std::next(scope->varlist.begin(), argn); if (it->valueType()) @@ -3307,10 +3308,11 @@ static std::vector getLifetimeTokens(const Token* tok, return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1), !(astIsContainer(vartok) && Token::simpleMatch(vartok->astParent(), "["))); } - } else if (Token::simpleMatch(tok, "{") && getArgumentStart(tok) && !Token::simpleMatch(getArgumentStart(tok), ",") && getArgumentStart(tok)->valueType()) { + } else if (Token::simpleMatch(tok, "{") && getArgumentStart(tok) && + !Token::simpleMatch(getArgumentStart(tok), ",") && getArgumentStart(tok)->valueType()) { const Token* vartok = getArgumentStart(tok); auto vts = getParentValueTypes(tok); - for(const ValueType& vt:vts) { + for (const ValueType& vt : vts) { if (vt.isTypeEqual(vartok->valueType())) return getLifetimeTokens(vartok, escape, std::move(errorPath), pred, depth - 1); } From a1c59bcfba3f39aad1994b28d7d74efbfed278c8 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 11 May 2022 17:17:37 -0500 Subject: [PATCH 4/7] Use ast for number of arguments --- lib/astutils.cpp | 45 ++++++++++++++++++++++++++++++--------------- lib/astutils.h | 6 ++++-- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index ec466ccc770..7177320817c 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -147,6 +147,17 @@ std::vector astFlatten(Token* tok, const char* op) return result; } +nonneg int astCount(const Token* tok, const char* op, int depth) +{ + --depth; + if (!tok || depth < 0) + return 0; + if (tok->str() == op) + return astCount(tok->astOperand1(), op, depth) + astCount(tok->astOperand2(), op, depth); + else + return 1; +} + bool astHasToken(const Token* root, const Token * tok) { if (!root) @@ -2589,6 +2600,24 @@ bool isExpressionChanged(const Token* expr, const Token* start, const Token* end return result; } +const Token* getArgumentStart(const Token* ftok) +{ + const Token* tok = ftok; + if (Token::Match(tok, "%name% (|{")) + tok = ftok->next(); + if (!Token::Match(tok, "(|{|[")) + return nullptr; + const Token* startTok = tok->astOperand2(); + if (!startTok && tok->next() != tok->link()) + startTok = tok->astOperand1(); + return startTok; +} + +// int numberOfArguments(const Token *ftok) +// { +// return astCount(getArgumentStart(ftok), ","); +// } + int numberOfArguments(const Token *start) { int arguments=0; @@ -2603,23 +2632,9 @@ int numberOfArguments(const Token *start) return arguments; } -const Token* getArgumentStart(const Token* tok) -{ - const Token* startTok = tok->astOperand2(); - if (!startTok && tok->next() != tok->link()) - startTok = tok->astOperand1(); - return startTok; -} - std::vector getArguments(const Token *ftok) { - const Token* tok = ftok; - if (Token::Match(tok, "%name% (|{")) - tok = ftok->next(); - if (!Token::Match(tok, "(|{|[")) - return std::vector {}; - const Token* startTok = getArgumentStart(tok); - return astFlatten(startTok, ","); + return astFlatten(getArgumentStart(ftok), ","); } int getArgumentPos(const Variable* var, const Function* f) diff --git a/lib/astutils.h b/lib/astutils.h index feea3cf4666..e9f3958df8f 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -89,6 +89,8 @@ const Token* findExpression(const Token* start, const nonneg int exprid); std::vector astFlatten(const Token* tok, const char* op); std::vector astFlatten(Token* tok, const char* op); +nonneg int astCount(const Token* tok, const char* op, int depth = 100); + bool astHasToken(const Token* root, const Token * tok); bool astHasVar(const Token * tok, nonneg int varid); @@ -310,13 +312,13 @@ bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive = nullptr) bool isAliased(const Variable *var); -const Token* getArgumentStart(const Token* tok); +const Token* getArgumentStart(const Token* ftok); /** Determines the number of arguments - if token is a function call or macro * @param start token which is supposed to be the function/macro name. * \return Number of arguments */ -int numberOfArguments(const Token *start); +int numberOfArguments(const Token *ftok); /** * Get arguments (AST) From 6e5fb2ff8001d30a6aa166f09a3c9c811ea578ab Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 11 May 2022 17:22:44 -0500 Subject: [PATCH 5/7] Get number of arguments using ast --- lib/astutils.cpp | 10 +++++----- lib/astutils.h | 3 +++ lib/library.cpp | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 7177320817c..8b92cfd1cac 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2613,12 +2613,12 @@ const Token* getArgumentStart(const Token* ftok) return startTok; } -// int numberOfArguments(const Token *ftok) -// { -// return astCount(getArgumentStart(ftok), ","); -// } +int numberOfArguments(const Token *ftok) +{ + return astCount(getArgumentStart(ftok), ","); +} -int numberOfArguments(const Token *start) +int numberOfArgumentsWithoutAst(const Token *start) { int arguments=0; const Token* const openBracket = start->next(); diff --git a/lib/astutils.h b/lib/astutils.h index e9f3958df8f..4b5e58d9604 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -320,6 +320,9 @@ const Token* getArgumentStart(const Token* ftok); */ int numberOfArguments(const Token *ftok); +/// Get number of arguments without using AST +int numberOfArgumentsWithoutAst(const Token *start); + /** * Get arguments (AST) */ diff --git a/lib/library.cpp b/lib/library.cpp index 793ec02bfa3..320ed6c4b31 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -1197,7 +1197,7 @@ bool Library::isNotLibraryFunction(const Token *ftok) const bool Library::matchArguments(const Token *ftok, const std::string &functionName) const { - const int callargs = numberOfArguments(ftok); + const int callargs = numberOfArgumentsWithoutAst(ftok); const std::unordered_map::const_iterator it = functions.find(functionName); if (it == functions.cend()) return (callargs == 0); From ecfb4b1da863b2c2b502580f8c0dd2dfef3f7fc7 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 11 May 2022 17:26:34 -0500 Subject: [PATCH 6/7] Skip aggregate constructor when there are too many arguments --- lib/valueflow.cpp | 2 +- test/testvalueflow.cpp | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index af2733068e6..39038e549ae 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -335,7 +335,7 @@ static std::vector getParentValueTypes(const Token* tok, const Scope* scope = t->classScope; // Check for aggregate constructors if (scope && scope->numConstructors == 0 && t->derivedFrom.empty() && - (t->isClassType() || t->isStructType())) { + (t->isClassType() || t->isStructType()) && numberOfArguments(ftok) < scope->varlist.size()) { assert(argn < scope->varlist.size()); auto it = std::next(scope->varlist.begin(), argn); if (it->valueType()) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index c3bb4d39982..fa1f445f741 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -6519,6 +6519,20 @@ class TestValueFlow : public TestFixture { code = "void f(const char * const x) { !!system(x); }\n"; valueOfTok(code, "x"); + + code = "struct struct1 {\n" + " int i1;\n" + " int i2;\n" + "};\n" + "struct struct2 {\n" + " char c1;\n" + " struct1 is1;\n" + " char c2[4];\n" + "};\n" + "void f() {\n" + " struct2 a = { 1, 2, 3, {4,5,6,7} }; \n" + "}\n"; + valueOfTok(code, "a"); } void valueFlowHang() { From 4f31b62136b5379e7dc461c92ed4a18cd67d18fd Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 11 May 2022 17:27:03 -0500 Subject: [PATCH 7/7] Format --- lib/astutils.cpp | 10 ++++------ lib/astutils.h | 4 ++-- test/testvalueflow.cpp | 22 +++++++++++----------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 8b92cfd1cac..cebce878107 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -153,7 +153,7 @@ nonneg int astCount(const Token* tok, const char* op, int depth) if (!tok || depth < 0) return 0; if (tok->str() == op) - return astCount(tok->astOperand1(), op, depth) + astCount(tok->astOperand2(), op, depth); + return astCount(tok->astOperand1(), op, depth) + astCount(tok->astOperand2(), op, depth); else return 1; } @@ -2613,12 +2613,11 @@ const Token* getArgumentStart(const Token* ftok) return startTok; } -int numberOfArguments(const Token *ftok) -{ +int numberOfArguments(const Token* ftok) { return astCount(getArgumentStart(ftok), ","); } -int numberOfArgumentsWithoutAst(const Token *start) +int numberOfArgumentsWithoutAst(const Token* start) { int arguments=0; const Token* const openBracket = start->next(); @@ -2632,8 +2631,7 @@ int numberOfArgumentsWithoutAst(const Token *start) return arguments; } -std::vector getArguments(const Token *ftok) -{ +std::vector getArguments(const Token* ftok) { return astFlatten(getArgumentStart(ftok), ","); } diff --git a/lib/astutils.h b/lib/astutils.h index 4b5e58d9604..184bad41533 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -318,10 +318,10 @@ const Token* getArgumentStart(const Token* ftok); * @param start token which is supposed to be the function/macro name. * \return Number of arguments */ -int numberOfArguments(const Token *ftok); +int numberOfArguments(const Token* ftok); /// Get number of arguments without using AST -int numberOfArgumentsWithoutAst(const Token *start); +int numberOfArgumentsWithoutAst(const Token* start); /** * Get arguments (AST) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index fa1f445f741..f2b0492f2be 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -6521,17 +6521,17 @@ class TestValueFlow : public TestFixture { valueOfTok(code, "x"); code = "struct struct1 {\n" - " int i1;\n" - " int i2;\n" - "};\n" - "struct struct2 {\n" - " char c1;\n" - " struct1 is1;\n" - " char c2[4];\n" - "};\n" - "void f() {\n" - " struct2 a = { 1, 2, 3, {4,5,6,7} }; \n" - "}\n"; + " int i1;\n" + " int i2;\n" + "};\n" + "struct struct2 {\n" + " char c1;\n" + " struct1 is1;\n" + " char c2[4];\n" + "};\n" + "void f() {\n" + " struct2 a = { 1, 2, 3, {4,5,6,7} }; \n" + "}\n"; valueOfTok(code, "a"); }