From ab24e3a3c85d6ec4a1821a130fde623618ec52a4 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Thu, 6 Apr 2023 18:46:45 +0200 Subject: [PATCH] Fix remaining example from #11599, FN #11646, fix crash (#4929) * Fix remainig example from #11599 * Fix FP, new warnings * More warnings * Use getTokenArgumentFunction() * Fix crash * Fix #11646 constParameter not reported with "const * const" parameter * Fix test * Fix new warnings * Add suppression * Add const, fix suppression --- lib/astutils.cpp | 3 --- lib/checkother.cpp | 27 ++++++++++++++++----------- lib/templatesimplifier.cpp | 4 ++-- lib/templatesimplifier.h | 4 ++-- lib/valueflow.cpp | 18 +++++++++--------- test/testother.cpp | 27 +++++++++++++++++++++++++++ test/testuninitvar.cpp | 10 +++------- 7 files changed, 59 insertions(+), 34 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 0b50bf399..b15c8e359 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2409,9 +2409,6 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti if (indirect > 0) { if (arg->isPointer() && !(arg->valueType() && arg->valueType()->isConst(indirect))) return true; - // If const is applied to the pointer, then the value can still be modified - if (Token::simpleMatch(arg->typeEndToken(), "* const")) - return true; if (arg->isArray() || (!arg->isPointer() && (!arg->valueType() || arg->valueType()->type == ValueType::UNKNOWN_TYPE))) return true; } diff --git a/lib/checkother.cpp b/lib/checkother.cpp index d3b395e20..78b557dfb 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1391,14 +1391,6 @@ static bool isVariableMutableInInitializer(const Token* start, const Token * end return false; } -static const Token* isFuncArg(const Token* tok) { - while (Token::simpleMatch(tok, ",")) - tok = tok->astParent(); - if (Token::simpleMatch(tok, "(") && Token::Match(tok->astOperand1(), "%name% (")) - return tok->astOperand1(); - return nullptr; -} - void CheckOther::checkConstVariable() { if (!mSettings->severity.isEnabled(Severity::style) || mTokenizer->isC()) @@ -1481,7 +1473,8 @@ void CheckOther::checkConstVariable() } if (tok->isUnaryOp("&") && Token::Match(tok, "& %varid%", var->declarationId())) { const Token* opTok = tok->astParent(); - if (opTok->isComparisonOp() || opTok->isAssignmentOp() || opTok->isCalculation()) { + int argn = -1; + if (opTok && (opTok->isComparisonOp() || opTok->isAssignmentOp() || opTok->isCalculation())) { if (opTok->isComparisonOp() || opTok->isCalculation()) { if (opTok->astOperand1() != tok) opTok = opTok->astOperand1(); @@ -1490,7 +1483,7 @@ void CheckOther::checkConstVariable() } if (opTok->valueType() && var->valueType() && opTok->valueType()->isConst(var->valueType()->pointer)) continue; - } else if (const Token* ftok = isFuncArg(opTok)) { + } else if (const Token* ftok = getTokenArgumentFunction(tok, argn)) { bool inconclusive{}; if (var->valueType() && !isVariableChangedByFunctionCall(ftok, var->valueType()->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive) continue; @@ -1572,6 +1565,7 @@ void CheckOther::checkConstPointer() const Token* const gparent = parent->astParent(); if (Token::Match(gparent, "%cop%") && !gparent->isUnaryOp("&") && !gparent->isUnaryOp("*")) continue; + int argn = -1; if (Token::simpleMatch(gparent, "return")) { const Function* function = gparent->scope()->function; if (function && (!Function::returnsReference(function) || Function::returnsConst(function))) @@ -1589,16 +1583,27 @@ void CheckOther::checkConstPointer() continue; } else if (Token::simpleMatch(gparent, "[") && gparent->astOperand2() == parent) continue; - else if (const Token* ftok = isFuncArg(gparent)) { + else if (const Token* ftok = getTokenArgumentFunction(parent, argn)) { bool inconclusive{}; if (!isVariableChangedByFunctionCall(ftok, vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive) continue; } } else { + int argn = -1; if (Token::Match(parent, "%oror%|%comp%|&&|?|!|-")) continue; else if (Token::simpleMatch(parent, "(") && Token::Match(parent->astOperand1(), "if|while")) continue; + else if (const Token* ftok = getTokenArgumentFunction(tok, argn)) { + if (ftok->function() && !parent->isCast()) { + const Variable* argVar = ftok->function()->getArgumentVar(argn); + if (argVar && argVar->valueType() && argVar->valueType()->isConst(vt->pointer)) { + bool inconclusive{}; + if (!isVariableChangedByFunctionCall(ftok, vt->pointer, var->declarationId(), mSettings, &inconclusive) && !inconclusive) + continue; + } + } + } } nonConstPointers.emplace_back(var); } diff --git a/lib/templatesimplifier.cpp b/lib/templatesimplifier.cpp index e87693cba..8ea4b8ccb 100644 --- a/lib/templatesimplifier.cpp +++ b/lib/templatesimplifier.cpp @@ -2409,7 +2409,7 @@ static Token *skipTernaryOp(Token *tok, const Token *backToken) return tok; } -void TemplateSimplifier::simplifyTemplateArgs(Token *start, Token *end) +void TemplateSimplifier::simplifyTemplateArgs(Token *start, const Token *end) { // start could be erased so use the token before start if available Token * first = (start && start->previous()) ? start->previous() : mTokenList.front(); @@ -2599,7 +2599,7 @@ static bool validTokenEnd(bool bounded, const Token *tok, const Token *backToken // TODO: This is not the correct class for simplifyCalculations(), so it // should be moved away. -bool TemplateSimplifier::simplifyCalculations(Token* frontToken, Token *backToken, bool isTemplate) +bool TemplateSimplifier::simplifyCalculations(Token* frontToken, const Token *backToken, bool isTemplate) { bool ret = false; const bool bounded = frontToken || backToken; diff --git a/lib/templatesimplifier.h b/lib/templatesimplifier.h index 672d60174..186170a6c 100644 --- a/lib/templatesimplifier.h +++ b/lib/templatesimplifier.h @@ -327,13 +327,13 @@ public: * @return true if modifications to token-list are done. * false if no modifications are done. */ - bool simplifyCalculations(Token* frontToken = nullptr, Token *backToken = nullptr, bool isTemplate = true); + bool simplifyCalculations(Token* frontToken = nullptr, const Token *backToken = nullptr, bool isTemplate = true); /** Simplify template instantiation arguments. * @param start first token of arguments * @param end token following last argument token */ - void simplifyTemplateArgs(Token *start, Token *end); + void simplifyTemplateArgs(Token *start, const Token *end); private: /** diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d9955d5a4..2c9b0afcf 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2028,7 +2028,7 @@ static Analyzer::Result valueFlowForward(Token* startToken, const Token* endToken, const Token* exprTok, ValueFlow::Value value, - TokenList* const tokenlist, + const TokenList* const tokenlist, const Settings* settings, SourceLocation loc = SourceLocation::current()) { @@ -2044,7 +2044,7 @@ static Analyzer::Result valueFlowForward(Token* startToken, const Token* endToken, const Token* exprTok, std::list values, - TokenList* const tokenlist, + const TokenList* const tokenlist, const Settings* settings, SourceLocation loc = SourceLocation::current()) { @@ -2073,7 +2073,7 @@ static Analyzer::Result valueFlowForward(Token* startToken, static Analyzer::Result valueFlowForwardRecursive(Token* top, const Token* exprTok, std::list values, - TokenList* const tokenlist, + const TokenList* const tokenlist, const Settings* settings, SourceLocation loc = SourceLocation::current()) { @@ -2091,7 +2091,7 @@ static void valueFlowReverse(Token* tok, const Token* const endToken, const Token* const varToken, std::list values, - TokenList* tokenlist, + const TokenList* tokenlist, const Settings* settings, SourceLocation loc = SourceLocation::current()) { @@ -2103,7 +2103,7 @@ static void valueFlowReverse(Token* tok, } // Deprecated -static void valueFlowReverse(TokenList* tokenlist, +static void valueFlowReverse(const TokenList* tokenlist, Token* tok, const Token* const varToken, ValueFlow::Value val, @@ -6999,7 +6999,7 @@ static void valueFlowForLoopSimplify(Token* const bodyStart, } } -static void valueFlowForLoopSimplifyAfter(Token* fortok, nonneg int varid, const MathLib::bigint num, TokenList* tokenlist, const Settings* settings) +static void valueFlowForLoopSimplifyAfter(Token* fortok, nonneg int varid, const MathLib::bigint num, const TokenList* tokenlist, const Settings* settings) { const Token *vartok = nullptr; for (const Token *tok = fortok; tok; tok = tok->next()) { @@ -7316,7 +7316,7 @@ static void valueFlowInjectParameter(TokenList* tokenlist, } } -static void valueFlowInjectParameter(TokenList* tokenlist, +static void valueFlowInjectParameter(const TokenList* tokenlist, const Settings* settings, const Variable* arg, const Scope* functionScope, @@ -7727,7 +7727,7 @@ static void addToErrorPath(ValueFlow::Value& value, const ValueFlow::Value& from }); } -static std::vector findAllUsages(const Variable* var, Token* start) +static std::vector findAllUsages(const Variable* var, Token* start) // cppcheck-suppress constParameter // FP { std::vector result; const Scope* scope = var->scope(); @@ -8713,7 +8713,7 @@ struct ContainerConditionHandler : ConditionHandler { } }; -static void valueFlowDynamicBufferSize(TokenList* tokenlist, SymbolDatabase* symboldatabase, const Settings* settings) +static void valueFlowDynamicBufferSize(const TokenList* tokenlist, SymbolDatabase* symboldatabase, const Settings* settings) { auto getBufferSizeFromAllocFunc = [&](const Token* funcTok) -> MathLib::bigint { MathLib::bigint sizeValue = -1; diff --git a/test/testother.cpp b/test/testother.cpp index 25c612121..7388329da 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -3455,6 +3455,21 @@ private: "[test.cpp:14]: (style) Parameter 's' can be declared as pointer to const\n", errout.str()); + check("void g(int, const int*);\n" + "void h(const int*);\n" + "void f(int* p) {\n" + " g(1, p);\n" + " h(p);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Parameter 'p' can be declared as pointer to const\n", + errout.str()); + + check("void f(int, const int*);\n" + "void f(int i, int* p) {\n" + " f(i, const_cast(p));\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + check("struct S { int a; };\n" "void f(std::vector& v, int b) {\n" " size_t n = v.size();\n" @@ -3465,6 +3480,18 @@ private: " }\n" "}\n"); ASSERT_EQUALS("[test.cpp:5]: (style) Variable 's' can be declared as reference to const\n", errout.str()); // don't crash + + check("void f(int& i) {\n" + " new (&i) int();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); // don't crash + + check("class C;\n" // #11646 + "void g(const C* const p);\n" + "void f(C* c) {\n" + " g(c);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Parameter 'c' can be declared as pointer to const\n", errout.str()); } void switchRedundantAssignmentTest() { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 67ffcda08..b6e116a27 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -5241,8 +5241,7 @@ private: " s.x = 42;\n" " bar(&s);\n" "}"); - ASSERT_EQUALS("[test.cpp:18] -> [test.cpp:12] -> [test.cpp:8]: (warning) Uninitialized variable: s->flag\n", - errout.str()); + ASSERT_EQUALS("[test.cpp:18]: (error) Uninitialized variable: &s.flag\n", errout.str()); // Ticket #2207 - False negative valueFlowUninit("void foo() {\n" @@ -6020,7 +6019,7 @@ private: " someType_t gVar;\n" " bar(&gVar);\n" "}"); - ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:5]: (warning) Uninitialized variable: p->flags\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (error) Uninitialized variable: &gVar\n", errout.str()); valueFlowUninit("typedef struct\n" "{\n" @@ -6783,10 +6782,7 @@ private: " abc.a = 1;\n" " setabc(123, &abc);\n" "}\n"); - TODO_ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Uninitialized variable: abc->b\n" - "[test.cpp:8] -> [test.cpp:3]: (error) Uninitialized variable: abc->c\n", - "[test.cpp:8] -> [test.cpp:3]: (warning) Uninitialized variable: abc->b\n", - errout.str()); + ASSERT_EQUALS("[test.cpp:8]: (error) Uninitialized variables: &abc.b, &abc.c\n", errout.str()); valueFlowUninit("struct S { int* p; };\n" // #10463 "void f(S* in) {\n"