From e54ad24d2c91b3626ea42694b5db75297da525ab Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Fri, 25 Mar 2022 11:32:16 +0100 Subject: [PATCH] Fix #10870 FN constStatement with arrays (#3904) --- Makefile | 2 +- lib/astutils.cpp | 5 +-- lib/astutils.h | 2 +- lib/checkother.cpp | 14 +++++++++ test/testincompletestatement.cpp | 54 ++++++++++++++++++++++++++------ 5 files changed, 64 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 7c83f4c93..44cb7d455 100644 --- a/Makefile +++ b/Makefile @@ -407,7 +407,7 @@ validateRules: $(libcppdir)/analyzerinfo.o: lib/analyzerinfo.cpp externals/tinyxml2/tinyxml2.h lib/analyzerinfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/path.h lib/platform.h lib/suppressions.h lib/utils.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/analyzerinfo.o $(libcppdir)/analyzerinfo.cpp -$(libcppdir)/astutils.o: lib/astutils.cpp lib/astutils.h lib/config.h lib/errortypes.h lib/importproject.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h +$(libcppdir)/astutils.o: lib/astutils.cpp lib/astutils.h lib/check.h lib/checkclass.h lib/config.h lib/errortypes.h lib/importproject.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/astutils.o $(libcppdir)/astutils.cpp $(libcppdir)/bughuntingchecks.o: lib/bughuntingchecks.cpp lib/astutils.h lib/bughuntingchecks.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/exprengine.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h diff --git a/lib/astutils.cpp b/lib/astutils.cpp index e34aa9586..141eb9cef 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -31,6 +31,7 @@ #include "utils.h" #include "valueflow.h" #include "valueptr.h" +#include "checkclass.h" #include #include @@ -1747,7 +1748,7 @@ bool isConstExpression(const Token *tok, const Library& library, bool pure, bool return isConstExpression(tok->astOperand1(), library, pure, cpp) && isConstExpression(tok->astOperand2(), library, pure, cpp); } -bool isWithoutSideEffects(bool cpp, const Token* tok) +bool isWithoutSideEffects(bool cpp, const Token* tok, bool checkArrayAccess) { if (!cpp) return true; @@ -1756,7 +1757,7 @@ bool isWithoutSideEffects(bool cpp, const Token* tok) tok = tok->astOperand2(); if (tok && tok->varId()) { const Variable* var = tok->variable(); - return var && (!var->isClass() || var->isPointer() || var->isStlType()); + return var && (!var->isClass() || var->isPointer() || (checkArrayAccess ? var->isStlType() && !var->isStlType(CheckClass::stl_containers_not_const) : var->isStlType())); } return true; } diff --git a/lib/astutils.h b/lib/astutils.h index 2107a4658..c29e59ee3 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -227,7 +227,7 @@ bool isConstFunctionCall(const Token* ftok, const Library& library); bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp); -bool isWithoutSideEffects(bool cpp, const Token* tok); +bool isWithoutSideEffects(bool cpp, const Token* tok, bool checkArrayAccess = false); bool isUniqueExpression(const Token* tok); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index b1e6d1846..212a27128 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1769,6 +1769,11 @@ static bool isConstStatement(const Token *tok, bool cpp) return tok->astParent() ? isConstStatement(tok->astOperand1(), cpp) && isConstStatement(tok->astOperand2(), cpp) : isConstStatement(tok->astOperand2(), cpp); if (Token::simpleMatch(tok, "?") && Token::simpleMatch(tok->astOperand2(), ":")) // ternary operator return isConstStatement(tok->astOperand1(), cpp) && isConstStatement(tok->astOperand2()->astOperand1(), cpp) && isConstStatement(tok->astOperand2()->astOperand2(), cpp); + if (Token::simpleMatch(tok, "[") && !Token::Match(tok->tokAt(-2), "%type% %name%") && isWithoutSideEffects(cpp, tok->astOperand1(), /*checkArrayAccess*/ true)) { + if (Token::simpleMatch(tok->astParent(), "[")) + return isConstStatement(tok->astOperand2(), cpp) && isConstStatement(tok->astParent(), cpp); + return isConstStatement(tok->astOperand2(), cpp); + } return false; } @@ -1801,6 +1806,13 @@ static bool isConstTop(const Token *tok) else return tok->astParent()->astOperand1() == tok; } + if (Token::simpleMatch(tok, "[")) { + const Token* bracTok = tok; + while (Token::simpleMatch(bracTok->astParent(), "[")) + bracTok = bracTok->astParent(); + if (!bracTok->astParent()) + return true; + } return false; } @@ -1894,6 +1906,8 @@ void CheckOther::constStatementError(const Token *tok, const std::string &type, msg = "Redundant code: Found unused result of ternary operator."; else if (tok->str() == "." && tok->tokType() == Token::Type::eOther) msg = "Redundant code: Found unused member access."; + else if (tok->str() == "[" && tok->tokType() == Token::Type::eExtendedOp) + msg = "Redundant code: Found unused array access."; else { reportError(tok, Severity::debug, "debug", "constStatementError not handled."); return; diff --git a/test/testincompletestatement.cpp b/test/testincompletestatement.cpp index a0bf24ada..9fdc9da76 100644 --- a/test/testincompletestatement.cpp +++ b/test/testincompletestatement.cpp @@ -481,15 +481,15 @@ private: " void g();\n" "};\n" "void f(const S& r, const S* p) {\n" - " r.b;\n" - " p->b;\n" - " S s;\n" - " (s.b);\n" - " T t, u[2];\n" - " t.s[1].b;\n" - " t.g();\n" - " u[0].g();\n" - " u[1].s[0].b;\n" + " r.b;\n" + " p->b;\n" + " S s;\n" + " (s.b);\n" + " T t, u[2];\n" + " t.s[1].b;\n" + " t.g();\n" + " u[0].g();\n" + " u[1].s[0].b;\n" "}\n"); ASSERT_EQUALS("[test.cpp:7]: (warning) Redundant code: Found unused member access.\n" "[test.cpp:8]: (warning) Redundant code: Found unused member access.\n" @@ -498,6 +498,42 @@ private: "[test.cpp:15]: (warning) Redundant code: Found unused member access.\n", errout.str()); + check("struct S { int a[2]{}; };\n" + "struct T { S s; };\n" + "void f() {\n" + " int i[2];\n" + " i[0] = 0;\n" + " i[0];\n" // <-- + " S s[1];\n" + " s[0].a[1];\n" // <-- + " T t;\n" + " t.s.a[1];\n" // <-- + " int j[2][2][1] = {};\n" + " j[0][0][0];\n" // <-- + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Redundant code: Found unused array access.\n" + "[test.cpp:8]: (warning) Redundant code: Found unused array access.\n" + "[test.cpp:10]: (warning) Redundant code: Found unused array access.\n" + "[test.cpp:12]: (warning) Redundant code: Found unused array access.\n", + errout.str()); + + check("void g() {\n" + " int j[2]{};\n" + " int k[2] = {};\n" + " int l[]{ 1, 2 };\n" + " int m[] = { 1, 2 };\n" + " h(0, j[0], 1);\n" + " C c{ 0, j[0], 1 };\n" + " c[0];\n" + " int j[2][2][2] = {};\n" + " j[h()][0][0];\n" + " j[0][h()][0];\n" + " j[0][0][h()];\n" + " std::map M;\n" + " M[\"abc\"];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + check("struct S { void* p; };\n" // #10875 "void f(S s) {\n" " delete (int*)s.p;\n"