From a954f24cfb82748f320ba693ed59c569c59e4759 Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 10 Jan 2023 13:20:25 +0100 Subject: [PATCH 1/3] Fix #11449 Function call not recognized with extra parentheses / FP nullPointer --- lib/valueflow.cpp | 7 ++++--- test/testnullpointer.cpp | 11 +++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b0f2f22b989..1e0ad2e1377 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); @@ -729,7 +730,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 e72194af0a0..966148daba4 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2838,14 +2838,21 @@ class TestNullPointer : public TestFixture { 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 From dac05c8f19f766a2ebae19911b7f44b2329ba9c9 Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 10 Jan 2023 14:46:32 +0100 Subject: [PATCH 2/3] Minor performance optimization --- lib/tokenize.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 234523c2663..f2da5318373 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)) From f35f60807a51a4276e6f358fdaa83ef394ca997a Mon Sep 17 00:00:00 2001 From: chrchr Date: Tue, 10 Jan 2023 14:48:13 +0100 Subject: [PATCH 3/3] Handle function call with extra parentheses --- lib/astutils.cpp | 6 ++++-- lib/library.cpp | 4 +++- lib/valueflow.cpp | 3 ++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index c97dee32cf7..cb791a8f44e 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -2824,8 +2824,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 a9044f1271d..6f90ddb661e 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/valueflow.cpp b/lib/valueflow.cpp index 1e0ad2e1377..1060234e5c5 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -604,7 +604,8 @@ static void setTokenValue(Token* tok, return !Token::simpleMatch(p, ","); }); // Ensure that the comma isnt 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; }