From e9a0d7979ed06c190e576ec083182e42758842a0 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sun, 1 May 2022 15:46:07 +0200 Subject: [PATCH] Fix #11014 FN redundantPointerOp / remove simplifyMulAndParens() (#4062) --- lib/checkleakautovar.cpp | 22 ++++++++++++-- lib/tokenize.cpp | 66 ---------------------------------------- lib/tokenize.h | 6 ---- test/testleakautovar.cpp | 39 ++++++++++++++++++++++++ test/testother.cpp | 6 ++++ test/testtokenize.cpp | 40 ------------------------ 6 files changed, 65 insertions(+), 114 deletions(-) diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index 13e8f8dac..c5ecacdd5 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -331,6 +331,11 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, if (Token::Match(tok, "const %type%")) tok = tok->tokAt(2); + while (tok->str() == "(") + tok = tok->next(); + while (tok->isUnaryOp("*") && tok->astOperand1()->isUnaryOp("&")) + tok = tok->astOperand1()->astOperand1(); + // parse statement, skip to last member const Token *varTok = tok; while (Token::Match(varTok, "%name% ::|. %name% !!(")) @@ -342,9 +347,22 @@ void CheckLeakAutoVar::checkScope(const Token * const startToken, while (Token::Match(ftok, "%name% :: %name%")) ftok = ftok->tokAt(2); + auto isAssignment = [](const Token* varTok) -> const Token* { + if (varTok->varId()) { + const Token* top = varTok; + while (top->astParent()) { + top = top->astParent(); + if (!Token::Match(top, "(|*|&|.")) + break; + } + if (top->str() == "=" && succeeds(top, varTok)) + return top; + } + return nullptr; + }; + // assignment.. - if (Token::Match(varTok, "%var% =")) { - const Token* const tokAssignOp = varTok->next(); + if (const Token* const tokAssignOp = isAssignment(varTok)) { // taking address of another variable.. if (Token::Match(tokAssignOp, "= %var% [+;]")) { diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 63ca7b162..7158d588d 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -2709,69 +2709,6 @@ bool Tokenizer::simplifyUsing() return substitute; } -void Tokenizer::simplifyMulAndParens() -{ - if (!list.front()) - return; - for (Token *tok = list.front()->tokAt(3); tok; tok = tok->next()) { - if (!tok->isName()) - continue; - //fix ticket #2784 - improved by ticket #3184 - int closedPars = 0; - Token *tokend = tok->next(); - Token *tokbegin = tok->previous(); - while (tokend && tokend->str() == ")") { - ++closedPars; - tokend = tokend->next(); - } - if (!tokend || !(tokend->isAssignmentOp())) - continue; - while (Token::Match(tokbegin, "&|(")) { - if (tokbegin->str() == "&") { - if (Token::Match(tokbegin->tokAt(-2), "[;{}&(] *")) { - //remove '* &' - tokbegin = tokbegin->tokAt(-2); - tokbegin->deleteNext(2); - } else if (Token::Match(tokbegin->tokAt(-3), "[;{}&(] * (")) { - if (closedPars == 0) - break; - --closedPars; - //remove ')' - tok->deleteNext(); - //remove '* ( &' - tokbegin = tokbegin->tokAt(-3); - tokbegin->deleteNext(3); - } else - break; - } else if (tokbegin->str() == "(") { - if (closedPars == 0) - break; - - //find consecutive opening parentheses - int openPars = 0; - while (tokbegin && tokbegin->str() == "(" && openPars <= closedPars) { - ++openPars; - tokbegin = tokbegin->previous(); - } - if (!tokbegin || openPars > closedPars) - break; - - if ((openPars == closedPars && Token::Match(tokbegin, "[;{}]")) || - Token::Match(tokbegin->tokAt(-2), "[;{}&(] * &") || - Token::Match(tokbegin->tokAt(-3), "[;{}&(] * ( &")) { - //remove the excessive parentheses around the variable - while (openPars > 0) { - tok->deleteNext(); - tokbegin->deleteNext(); - --closedPars; - --openPars; - } - } else - break; - } - } - } -} bool Tokenizer::createTokens(std::istream &code, const std::string& FileName) @@ -5039,9 +4976,6 @@ bool Tokenizer::simplifyTokenList1(const char FileName[]) // simplify labels and 'case|default'-like syntaxes simplifyLabelsCaseDefault(); - // simplify '[;{}] * & ( %any% ) =' to '%any% =' - simplifyMulAndParens(); - if (!isC() && !mSettings->library.markupFile(FileName)) { findComplicatedSyntaxErrorsInTemplates(); } diff --git a/lib/tokenize.h b/lib/tokenize.h index df223575b..791d60ae3 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -202,12 +202,6 @@ public: */ static void eraseDeadCode(Token *begin, const Token *end); - /** - * Simplify '* & ( %name% ) =' or any combination of '* &' and '()' - * parentheses around '%name%' to '%name% =' - */ - void simplifyMulAndParens(); - /** * Calculates sizeof value for given type. * @param type Token which will contain e.g. "int", "*", or string. diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp index 1c8b1b914..8904f700d 100644 --- a/test/testleakautovar.cpp +++ b/test/testleakautovar.cpp @@ -92,6 +92,7 @@ private: TEST_CASE(assign20); // #9187 TEST_CASE(assign21); // #10186 TEST_CASE(assign22); // #9139 + TEST_CASE(assign23); TEST_CASE(isAutoDealloc); @@ -463,6 +464,44 @@ private: ASSERT_EQUALS("[test.cpp:3]: (error) Memory leak: p\n", errout.str()); } + void assign23() { + Settings s = settings; + LOAD_LIB_2(settings.library, "posix.cfg"); + check("void f() {\n" + " int n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14;\n" + " *&n1 = open(\"xx.log\", O_RDONLY);\n" + " *&(n2) = open(\"xx.log\", O_RDONLY);\n" + " *(&n3) = open(\"xx.log\", O_RDONLY);\n" + " *&*&n4 = open(\"xx.log\", O_RDONLY);\n" + " *&*&*&(n5) = open(\"xx.log\", O_RDONLY);\n" + " *&*&(*&n6) = open(\"xx.log\", O_RDONLY);\n" + " *&*(&*&n7) = open(\"xx.log\", O_RDONLY);\n" + " *(&*&n8) = open(\"xx.log\", O_RDONLY);\n" + " *&(*&*&(*&n9)) = open(\"xx.log\", O_RDONLY);\n" + " (n10) = open(\"xx.log\", O_RDONLY);\n" + " ((n11)) = open(\"xx.log\", O_RDONLY);\n" + " ((*&n12)) = open(\"xx.log\", O_RDONLY);\n" + " *(&(*&n13)) = open(\"xx.log\", O_RDONLY);\n" + " ((*&(*&n14))) = open(\"xx.log\", O_RDONLY);\n" + "}\n", true); + ASSERT_EQUALS("[test.cpp:17]: (error) Resource leak: n1\n" + "[test.cpp:17]: (error) Resource leak: n2\n" + "[test.cpp:17]: (error) Resource leak: n3\n" + "[test.cpp:17]: (error) Resource leak: n4\n" + "[test.cpp:17]: (error) Resource leak: n5\n" + "[test.cpp:17]: (error) Resource leak: n6\n" + "[test.cpp:17]: (error) Resource leak: n7\n" + "[test.cpp:17]: (error) Resource leak: n8\n" + "[test.cpp:17]: (error) Resource leak: n9\n" + "[test.cpp:17]: (error) Resource leak: n10\n" + "[test.cpp:17]: (error) Resource leak: n11\n" + "[test.cpp:17]: (error) Resource leak: n12\n" + "[test.cpp:17]: (error) Resource leak: n13\n" + "[test.cpp:17]: (error) Resource leak: n14\n", + errout.str()); + settings = s; + } + void isAutoDealloc() { check("void f() {\n" " char *p = new char[100];" diff --git a/test/testother.cpp b/test/testother.cpp index 541a2b6f0..8a0d2e766 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -8774,6 +8774,12 @@ private: "[test.cpp:4]: (style) Redundant pointer operation on 'value' - it's already a variable.\n", errout.str()); + check("void f(int& a, int b) {\n" + " *(&a) = b;\n" + "}\n", nullptr, false, true); + ASSERT_EQUALS("[test.cpp:2]: (style) Redundant pointer operation on 'a' - it's already a variable.\n", + errout.str()); + check("void f(int**& p) {}\n", nullptr, false, true); ASSERT_EQUALS("", errout.str()); diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 76ffa1bac..ef4ee793f 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -187,7 +187,6 @@ private: TEST_CASE(tokenize_double); TEST_CASE(tokenize_strings); - TEST_CASE(simplifyMulAndParens); // Ticket #2784 + #3184 TEST_CASE(simplifyStructDecl); @@ -1949,45 +1948,6 @@ private: "}", tokenizeAndStringify(code)); } - void simplifyMulAndParens() { - // (error) Resource leak - const char code[] = "void f() {" - " *&n1=open();" - " *&(n2)=open();" - " *(&n3)=open();" - " *&*&n4=open();" - " *&*&*&(n5)=open();" - " *&*&(*&n6)=open();" - " *&*(&*&n7)=open();" - " *(&*&n8)=open();" - " *&(*&*&(*&n9))=open();" - " (n10) = open();" - " ((n11)) = open();" - " ((*&n12))=open();" - " *(&(*&n13))=open();" - " ((*&(*&n14)))=open();" - " ((*&(*&n15)))+=10;" - "}"; - const char expected[] = "void f ( ) {" - " n1 = open ( ) ;" - " n2 = open ( ) ;" - " n3 = open ( ) ;" - " n4 = open ( ) ;" - " n5 = open ( ) ;" - " n6 = open ( ) ;" - " n7 = open ( ) ;" - " n8 = open ( ) ;" - " n9 = open ( ) ;" - " n10 = open ( ) ;" - " n11 = open ( ) ;" - " n12 = open ( ) ;" - " n13 = open ( ) ;" - " n14 = open ( ) ;" - " n15 += 10 ; " - "}"; - ASSERT_EQUALS(expected, tokenizeAndStringify(code)); - } - void simplifyStructDecl() { const char code[] = "const struct A { int a; int b; } a;"; ASSERT_EQUALS("struct A { int a ; int b ; } ; const struct A a ;", tokenizeAndStringify(code));