From 3076f9def1b6d0b434a0b70c705eac648d7cbccb Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Wed, 18 Jan 2023 20:38:37 +0100 Subject: [PATCH] Fix #11449 Function call not recognized with extra parentheses / FP nullPointer (#4703) --- lib/astutils.cpp | 6 ++++-- lib/library.cpp | 4 +++- lib/tokenize.cpp | 2 +- lib/valueflow.cpp | 10 ++++++---- test/testnullpointer.cpp | 11 +++++++++-- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index e365a4268..cafc7d6d6 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2833,8 +2833,10 @@ int numberOfArguments(const Token* ftok) { int numberOfArgumentsWithoutAst(const Token* start) { - int arguments=0; - const Token* const openBracket = start->next(); + int arguments = 0; + const Token* openBracket = start->next(); + while (Token::simpleMatch(openBracket, ")")) + openBracket = openBracket->next(); if (openBracket && openBracket->str()=="(" && openBracket->next() && openBracket->next()->str()!=")") { const Token* argument=openBracket->next(); while (argument) { diff --git a/lib/library.cpp b/lib/library.cpp index a9044f127..6f90ddb66 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -982,7 +982,7 @@ std::string Library::getFunctionName(const Token *ftok, bool *error) const std::string Library::getFunctionName(const Token *ftok) const { - if (!Token::Match(ftok, "%name% (") && (ftok->strAt(-1) != "&" || ftok->previous()->astOperand2())) + if (!Token::Match(ftok, "%name% )| (") && (ftok->strAt(-1) != "&" || ftok->previous()->astOperand2())) return ""; // Lookup function name using AST.. @@ -1228,6 +1228,8 @@ bool Library::isNotLibraryFunction(const Token *ftok) const bool Library::matchArguments(const Token *ftok, const std::string &functionName) const { + if (functionName.empty()) + return false; const int callargs = numberOfArgumentsWithoutAst(ftok); const std::unordered_map::const_iterator it = functions.find(functionName); if (it == functions.cend()) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 1858f0745..2d6a698c5 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -8163,7 +8163,7 @@ void Tokenizer::simplifyDeclspec() void Tokenizer::simplifyAttribute() { for (Token *tok = list.front(); tok; tok = tok->next()) { - if (Token::Match(tok, "%type% (") && !mSettings->library.isNotLibraryFunction(tok)) { + if (!tok->isKeyword() && Token::Match(tok, "%type% (") && !mSettings->library.isNotLibraryFunction(tok)) { if (mSettings->library.isFunctionConst(tok->str(), true)) tok->isAttributePure(true); if (mSettings->library.isFunctionConst(tok->str(), false)) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ab2989579..7adb63586 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -459,7 +459,7 @@ void combineValueProperties(const ValueFlow::Value &value1, const ValueFlow::Val result->path = value1.path; } -static const Token *getCastTypeStartToken(const Token *parent) +static const Token *getCastTypeStartToken(const Token *parent, const Settings* settings) { // TODO: This might be a generic utility function? if (!Token::Match(parent, "{|(")) @@ -470,7 +470,8 @@ static const Token *getCastTypeStartToken(const Token *parent) return parent->astOperand1(); if (parent->str() != "(") return nullptr; - if (!parent->astOperand2() && Token::Match(parent,"( %name%")) + if (!parent->astOperand2() && Token::Match(parent, "( %name%") && + (parent->next()->isStandardType() || settings->library.isNotLibraryFunction(parent->next()))) return parent->next(); if (parent->astOperand2() && Token::Match(parent->astOperand1(), "const_cast|dynamic_cast|reinterpret_cast|static_cast <")) return parent->astOperand1()->tokAt(2); @@ -603,7 +604,8 @@ static void setTokenValue(Token* tok, return !Token::simpleMatch(p, ","); }); // Ensure that the comma isn't a function call - if (!callParent || (!Token::Match(callParent->previous(), "%name%|> (") && !Token::simpleMatch(callParent, "{"))) { + if (!callParent || (!Token::Match(callParent->previous(), "%name%|> (") && !Token::simpleMatch(callParent, "{") && + (!Token::Match(callParent, "( %name%") || settings->library.isNotLibraryFunction(callParent->next())))) { setTokenValue(parent, std::move(value), settings); return; } @@ -729,7 +731,7 @@ static void setTokenValue(Token* tok, } // cast.. - if (const Token *castType = getCastTypeStartToken(parent)) { + if (const Token *castType = getCastTypeStartToken(parent, settings)) { if (contains({ValueFlow::Value::ValueType::INT, ValueFlow::Value::ValueType::SYMBOLIC}, value.valueType) && Token::simpleMatch(parent->astOperand1(), "dynamic_cast")) return; diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index e9d25759d..bd9a595ad 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2838,14 +2838,21 @@ private: ASSERT_EQUALS("[test.cpp:7]: (warning) Possible null pointer dereference: p\n", errout.str()); } - void nullpointer_cast() { // #4692 - check("char *nasm_skip_spaces(const char *p) {\n" + void nullpointer_cast() { + check("char *nasm_skip_spaces(const char *p) {\n" // #4692 " if (p)\n" " while (*p && nasm_isspace(*p))\n" " p++;\n" " return p;\n" "}"); ASSERT_EQUALS("", errout.str()); + + check("void f(char* origin) {\n" // #11449 + " char* cp = (strchr)(origin, '\\0');\n" + " if (cp[-1] != '/')\n" + " *cp++ = '/';\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void nullpointer_castToVoid() { // #3771