Merge branch 'danmar:main' into checkEvaluatingOrder

danmar · May 15, 2024 · f724612 · f724612
2 parents 05e8840 + 994c015
commit f724612
Show file tree

Hide file tree

Showing 18 changed files with 203 additions and 10 deletions.
diff --git a/.github/workflows/scriptcheck.yml b/.github/workflows/scriptcheck.yml
@@ -47,7 +47,7 @@ jobs:
     runs-on: ubuntu-20.04
     strategy:
       matrix:
-        python-version: [3.5, 3.6, 3.7, 3.8, 3.9, '3.10', '3.11', '3.12']
+        python-version: [3.6, 3.7, 3.8, 3.9, '3.10', '3.11', '3.12']
         include:
           - python-version: '3.12'
             python-latest: true

diff --git a/cfg/std.cfg b/cfg/std.cfg
@@ -8201,6 +8201,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
     <noreturn>false</noreturn>
     <use-retval/>
     <leak-ignore/>
+    <container yields="iterator"/>
     <arg nr="1" direction="in">
       <not-uninit/>
     </arg>
@@ -8215,6 +8216,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
     <noreturn>false</noreturn>
     <use-retval/>
     <leak-ignore/>
+    <container yields="iterator"/>
     <arg nr="1" direction="in">
       <not-uninit/>
     </arg>

diff --git a/lib/checkassert.cpp b/lib/checkassert.cpp
@@ -70,8 +70,9 @@ void CheckAssert::assertWithSideEffects()
                         continue;
                     if (tmp->str() == "get" && Token::simpleMatch(tmp->astParent(), ".") && astIsSmartPointer(tmp->astParent()->astOperand1()))
                         continue;
-                    if (f->containerYield == Library::Container::Yield::START_ITERATOR || // bailout for std::begin/end
-                        f->containerYield == Library::Container::Yield::END_ITERATOR)
+                    if (f->containerYield == Library::Container::Yield::START_ITERATOR || // bailout for std::begin/end/prev/next
+                        f->containerYield == Library::Container::Yield::END_ITERATOR ||
+                        f->containerYield == Library::Container::Yield::ITERATOR)
                         continue;
                     sideEffectInAssertError(tmp, mSettings->library.getFunctionName(tmp));
                 }

diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
@@ -716,7 +716,7 @@ void CheckCondition::multiCondition2()
 
                     // Condition..
                     const Token *cond2 = tok->str() == "if" ? condStartToken->astOperand2() : condStartToken->astOperand1();
-                    const bool isReturnVar = (tok->str() == "return" && !Token::Match(cond2, "%cop%"));
+                    const bool isReturnVar = (tok->str() == "return" && (!Token::Match(cond2, "%cop%") || (cond2 && cond2->isUnaryOp("!"))));
 
                     ErrorPath errorPath;
 

diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp
@@ -401,7 +401,7 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken,
             if (Token::simpleMatch(tokAssignOp->astOperand1(), "."))
                 continue;
             // taking address of another variable..
-            if (Token::Match(tokAssignOp, "= %var% [+;]")) {
+            if (Token::Match(tokAssignOp, "= %var% +|;|?|%comp%")) {
                 if (varTok->tokAt(2)->varId() != varTok->varId()) {
                     // If variable points at allocated memory => error
                     leakIfAllocated(varTok, varInfo);

diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp
@@ -566,6 +566,7 @@ const ::Type * clangimport::AstNode::addTypeTokens(TokenList &tokenList, const s
     if (type.find('(') != std::string::npos)
         type.erase(type.find('('));
 
+    // TODO: put in a helper?
     std::stack<Token *> lpar;
     for (const std::string &s: splitString(type)) {
         Token *tok = addtoken(tokenList, s, false);

diff --git a/lib/importproject.cpp b/lib/importproject.cpp
@@ -33,6 +33,7 @@
 #include <iostream>
 #include <iterator>
 #include <sstream>
+#include <stack>
 #include <unordered_set>
 #include <utility>
 
@@ -568,6 +569,22 @@ namespace {
             TokenList tokenlist(&s);
             std::istringstream istr(c);
             tokenlist.createTokens(istr, Standards::Language::C); // TODO: check result
+            // TODO: put in a helper
+            // generate links
+            {
+                std::stack<Token*> lpar;
+                for (Token* tok2 = tokenlist.front(); tok2; tok2 = tok2->next()) {
+                    if (tok2->str() == "(")
+                        lpar.push(tok2);
+                    else if (tok2->str() == ")") {
+                        if (lpar.empty())
+                            break;
+                        Token::createMutualLinks(lpar.top(), tok2);
+                        lpar.pop();
+                    }
+                }
+            }
+            tokenlist.createAst();
             for (const Token *tok = tokenlist.front(); tok; tok = tok->next()) {
                 if (tok->str() == "(" && tok->astOperand1() && tok->astOperand2()) {
                     // TODO: this is wrong - it is Contains() not Equals()

diff --git a/lib/library.cpp b/lib/library.cpp
@@ -1774,6 +1774,7 @@ std::shared_ptr<Token> createTokenFromExpression(const std::string& returnValue,
             return nullptr;
     }
 
+    // TODO: put in a helper?
     // combine operators, set links, etc..
     std::stack<Token*> lpar;
     for (Token* tok2 = tokenList->front(); tok2; tok2 = tok2->next()) {

diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp
@@ -1570,7 +1570,7 @@ namespace {
         if (!tok->astParent() || tok->astParent()->isControlFlowKeyword())
             return;
         const Token* op1 = tok->astParent()->astOperand1();
-        if (op1 && op1->exprId() == 0)
+        if (op1 && op1->exprId() == 0 && !Token::Match(op1, "[{[]"))
             return;
         const Token* op2 = tok->astParent()->astOperand2();
         if (op2 && op2->exprId() == 0 &&

diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp
@@ -102,7 +102,7 @@ const Token * Tokenizer::isFunctionHead(const Token *tok, const std::string &end
         tok = tok->link();
     if (tok->str() != ")")
         return nullptr;
-    if (!tok->isCpp() && !Token::Match(tok->link()->previous(), "%name%|)"))
+    if (!tok->isCpp() && !Token::Match(tok->link()->previous(), "%name%|(|)"))
         return nullptr;
     if (Token::Match(tok, ") ;|{|[")) {
         tok = tok->next();
@@ -5167,12 +5167,15 @@ void Tokenizer::setVarIdPass2()
                     if (tok2->strAt(-1) == ")")
                         setVarIdClassFunction(scopeName2 + classname, tok2, tok2->link(), thisClassVars, structMembers, mVarId);
                     tok2 = tok2->link();
-                } else if (Token::Match(tok2, "( %name%|)") && !Token::Match(tok2->link(), "(|[")) {
+                } else if (Token::Match(tok2, "( %name%|)")) {
                     tok2 = tok2->link();
 
                     // Skip initialization list
-                    if (Token::simpleMatch(tok2, ") :"))
+                    if (Token::simpleMatch(tok2, ") :")) {
                         tok2 = skipInitializerList(tok2->next());
+                        if (Token::simpleMatch(tok2, "{"))
+                            tok2 = tok2->link();
+                    }
                 }
             }
 

diff --git a/lib/tokenlist.cpp b/lib/tokenlist.cpp
@@ -303,6 +303,7 @@ Token *TokenList::copyTokens(Token *dest, const Token *first, const Token *last,
 
 void TokenList::insertTokens(Token *dest, const Token *src, nonneg int n)
 {
+    // TODO: put the linking in a helper?
     std::stack<Token *> link;
 
     while (n > 0) {

diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp
@@ -5041,3 +5041,9 @@ void assertWithSideEffect_std_begin(const std::vector<std::string>& v) {
         return a.size() < b.size();
     })); // cppcheck-suppress checkLibraryNoReturn
 }
+
+void assertWithSideEffect_std_prev_next(const std::vector<int>& v, std::vector<int>::const_iterator it) {
+    assert(std::prev(it, 1) == v.begin());
+    // cppcheck-suppress checkLibraryNoReturn
+    assert(std::next(it, 1) == v.end());
+}
diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp
@@ -2442,7 +2442,7 @@ class TestAutoVariables : public TestFixture {
               "    return std::next(it);\n"
               "}");
         ASSERT_EQUALS(
-            "[test.cpp:3] -> [test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning object that points to local variable 'x' that will be invalid when returning.\n",
+            "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 'x' that will be invalid when returning.\n",
             errout_str());
 
         check("auto f() {\n"

diff --git a/test/testcondition.cpp b/test/testcondition.cpp
@@ -2129,6 +2129,15 @@ class TestCondition : public TestFixture {
               "    bool m_value = false;\n"
               "};");
         ASSERT_EQUALS("", errout_str());
+
+        // #12725
+        check("bool f(bool b) {\n"
+              "    if (b)\n"
+              "        return !b;\n"
+              "    b = g();\n"
+              "    return b;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Return value '!b' is always false\n", errout_str());
     }
 
     void oppositeInnerConditionPointers() {

diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp
@@ -83,6 +83,7 @@ class TestLeakAutoVar : public TestFixture {
         TEST_CASE(assign23);
         TEST_CASE(assign24); // #7440
         TEST_CASE(assign25);
+        TEST_CASE(assign26);
 
         TEST_CASE(isAutoDealloc);
 
@@ -620,6 +621,20 @@ class TestLeakAutoVar : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
 
+    void assign26() {
+        check("void f(int*& x) {\n" // #8235
+              "    int* p = (int*)malloc(10);\n"
+              "    x = p ? p : nullptr;\n"
+              "}", true);
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f(int*& x) {\n"
+              "    int* p = (int*)malloc(10);\n"
+              "    x = p != nullptr ? p : nullptr;\n"
+              "}", true);
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void isAutoDealloc() {
         check("void f() {\n"
               "    char *p = new char[100];"

diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp
@@ -2493,6 +2493,15 @@ class TestMemleakNoVar : public TestFixture {
               "}");
         ASSERT_EQUALS("", errout_str());
 
+        check("int f( void ) {\n" // FP: #11246
+              "  void* address = malloc( 1 );\n"
+              "  int ok = address != 0;\n"
+              "  if ( ok )\n"
+              "    free( address ), address = 0;\n"
+              "  return 0;\n"
+              "} ");
+        ASSERT_EQUALS("", errout_str());
+
         check("char** x(const char* str) {\n"
               "    char* ptr[] = { malloc(10), malloc(5), strdup(str) };\n"
               "    return ptr;\n"
@@ -2740,6 +2749,16 @@ class TestMemleakNoVar : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
     void resourceLeak() {
+        check("bool f(const char* name)\n" // FP: #12253
+              "{\n"
+              "    FILE* fp = fopen(name, \"r\");\n"
+              "    bool result = (fp == nullptr);\n"
+              "    if (result) return !result;\n"
+              "    fclose(fp);\n"
+              "    return result;\n"
+              "}");
+        ASSERT_EQUALS("", errout_str());
+
         check("void foo() {\n"
               "  fopen(\"file.txt\", \"r\");\n"
               "}");

diff --git a/test/testtokenlist.cpp b/test/testtokenlist.cpp
@@ -26,6 +26,7 @@
 #include "tokenlist.h"
 
 #include <sstream>
+#include <stack>
 #include <string>
 
 #include <simplecpp.h>
@@ -43,6 +44,7 @@ class TestTokenList : public TestFixture {
         TEST_CASE(inc);
         TEST_CASE(isKeyword);
         TEST_CASE(notokens);
+        TEST_CASE(ast1);
     }
 
     // inspired by #5895
@@ -161,6 +163,30 @@ class TestTokenList : public TestFixture {
         TokenList tokenlist(&settingsDefault);
         tokenlist.createTokens(std::move(tokensP)); // do not assert
     }
+
+    void ast1() const {
+        const std::string s = "('Release|x64' == 'Release|x64');";
+
+        TokenList tokenlist(&settings);
+        std::istringstream istr(s);
+        tokenlist.createTokens(istr, Standards::Language::C);
+        // TODO: put this logic in TokenList
+        // generate links
+        {
+            std::stack<Token*> lpar;
+            for (Token* tok2 = tokenlist.front(); tok2; tok2 = tok2->next()) {
+                if (tok2->str() == "(")
+                    lpar.push(tok2);
+                else if (tok2->str() == ")") {
+                    if (lpar.empty())
+                        break;
+                    Token::createMutualLinks(lpar.top(), tok2);
+                    lpar.pop();
+                }
+            }
+        }
+        tokenlist.createAst(); // do not crash
+    }
 };
 
 REGISTER_TEST(TestTokenList)