From b04bf7396f253f545bc831ea401cbae63f6359ff Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sat, 20 Aug 2022 20:52:10 +0200 Subject: [PATCH] Fix #7515 New check: Not needed c_str() operation (#4371) --- lib/astutils.cpp | 6 ++++++ lib/astutils.h | 1 + lib/checkstl.cpp | 28 +++++++++++++++++++++++++--- lib/checkstl.h | 1 + lib/symboldatabase.cpp | 2 +- test/teststl.cpp | 15 +++++++++++++++ 6 files changed, 49 insertions(+), 4 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 69c1ceefd..e30798d45 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -380,6 +380,12 @@ bool isVariableDecl(const Token* tok) return false; } +bool isStlStringType(const Token* tok) +{ + return Token::Match(tok, "std :: string|wstring|u16string|u32string !!::") || + (Token::simpleMatch(tok, "std :: basic_string <") && !Token::simpleMatch(tok->linkAt(3), "> ::")); +} + bool isTemporary(bool cpp, const Token* tok, const Library* library, bool unknown) { if (!tok) diff --git a/lib/astutils.h b/lib/astutils.h index 1eeffb640..d65bf05a8 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -162,6 +162,7 @@ std::string astCanonicalType(const Token *expr); const Token * astIsVariableComparison(const Token *tok, const std::string &comp, const std::string &rhs, const Token **vartok=nullptr); bool isVariableDecl(const Token* tok); +bool isStlStringType(const Token* tok); bool isTemporary(bool cpp, const Token* tok, const Library* library, bool unknown = false); diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index f9fc7271b..0a772b674 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -2009,6 +2009,18 @@ void CheckStl::string_c_str() ((Token::Match(tok->previous(), "%var% + %var% . c_str|data ( )") && tok->previous()->variable() && tok->previous()->variable()->isStlStringType()) || (Token::Match(tok->tokAt(-5), "%var% . c_str|data ( ) + %var%") && tok->tokAt(-5)->variable() && tok->tokAt(-5)->variable()->isStlStringType()))) { string_c_strConcat(tok); + } else if (printPerformance && Token::simpleMatch(tok, "<<") && tok->astOperand2() && Token::simpleMatch(tok->astOperand2()->astOperand1(), ". c_str ( )")) { + const Token* str = tok->astOperand2()->astOperand1()->astOperand1(); + if (Token::Match(str, "(|[")) + str = str->previous(); + if (str && ((str->variable() && str->variable()->isStlStringType()) || + (str->function() && isStlStringType(str->function()->retDef)))) { + const Token* strm = tok; + while (Token::simpleMatch(strm, "<<")) + strm = strm->astOperand1(); + if (strm && strm->variable() && strm->variable()->isStlType()) + string_c_strStream(tok); + } } // Using c_str() to get the return value is only dangerous if the function returns a char* @@ -2118,22 +2130,32 @@ void CheckStl::string_c_strParam(const Token* tok, nonneg int number) void CheckStl::string_c_strConstructor(const Token* tok) { - std::string msg = "Constructing a std::string from the result of c_str() is slow and redundant.\nSolve that by directly passing the string."; + std::string msg = "Constructing a std::string from the result of c_str() is slow and redundant.\n" + "Constructing a std::string from const char* requires a call to strlen(). Solve that by directly passing the string."; reportError(tok, Severity::performance, "stlcstrConstructor", msg, CWE704, Certainty::normal); } void CheckStl::string_c_strAssignment(const Token* tok) { - std::string msg = "Assigning the result of c_str() to a std::string is slow and redundant.\nSolve that by directly assigning the string."; + std::string msg = "Assigning the result of c_str() to a std::string is slow and redundant.\n" + "Assigning a const char* to a std::string requires a call to strlen(). Solve that by directly assigning the string."; reportError(tok, Severity::performance, "stlcstrAssignment", msg, CWE704, Certainty::normal); } void CheckStl::string_c_strConcat(const Token* tok) { - std::string msg = "Concatenating the result of c_str() and a std::string is slow and redundant.\nSolve that by directly concatenating the strings."; + std::string msg = "Concatenating the result of c_str() and a std::string is slow and redundant.\n" + "Concatenating a const char* with a std::string requires a call to strlen(). Solve that by directly concatenating the strings."; reportError(tok, Severity::performance, "stlcstrConcat", msg, CWE704, Certainty::normal); } +void CheckStl::string_c_strStream(const Token* tok) +{ + std::string msg = "Passing the result of c_str() to a stream is slow and redundant.\n" + "Passing a const char* to a stream requires a call to strlen(). Solve that by directly passing the string."; + reportError(tok, Severity::performance, "stlcstrStream", msg, CWE704, Certainty::normal); +} + //--------------------------------------------------------------------------- // //--------------------------------------------------------------------------- diff --git a/lib/checkstl.h b/lib/checkstl.h index 11e38e649..895082514 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -196,6 +196,7 @@ private: void string_c_strConstructor(const Token* tok); void string_c_strAssignment(const Token* tok); void string_c_strConcat(const Token* tok); + void string_c_strStream(const Token* tok); void outOfBoundsError(const Token *tok, const std::string &containerName, const ValueFlow::Value *containerSize, const std::string &index, const ValueFlow::Value *indexValue); void outOfBoundsIndexExpressionError(const Token *tok, const Token *index); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index f3c6d3f96..73bc722df 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -2187,7 +2187,7 @@ void Variable::evaluate(const Settings* settings) strtype += "::" + typeToken->strAt(2); setFlag(fIsClass, !lib->podtype(strtype) && !mTypeStartToken->isStandardType() && !isEnumType() && !isPointer() && !isReference() && strtype != "..."); setFlag(fIsStlType, Token::simpleMatch(mTypeStartToken, "std ::")); - setFlag(fIsStlString, isStlType() && (Token::Match(mTypeStartToken->tokAt(2), "string|wstring|u16string|u32string !!::") || (Token::simpleMatch(mTypeStartToken->tokAt(2), "basic_string <") && !Token::simpleMatch(mTypeStartToken->linkAt(3), "> ::")))); + setFlag(fIsStlString, ::isStlStringType(mTypeStartToken)); setFlag(fIsSmartPointer, lib->isSmartPointer(mTypeStartToken)); } if (mAccess == AccessControl::Argument) { diff --git a/test/teststl.cpp b/test/teststl.cpp index c988a3966..cd9609b21 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -4067,6 +4067,21 @@ private: " const double* const QM_R__ buf(v.data() + i);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("struct T { std::string g(); std::string a[1]; }\n" // #7515 + "void f(std::stringstream& strm, const std::string& s, T& t) {\n" + " strm << s.c_str();\n" + " strm << \"abc\" << s.c_str();\n" + " strm << \"abc\" << s.c_str() << \"def\";\n" + " strm << \"abc\" << t.g().c_str() << \"def\";\n" + " strm << t.a[0].c_str();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n" + "[test.cpp:4]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n" + "[test.cpp:5]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n" + "[test.cpp:6]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n" + "[test.cpp:7]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n", + errout.str()); } void uselessCalls() {